feat: per-job enable/disable checkboxes and interval editing in scheduler

[axisrow]

Summary

• Чекбоксы в таблице джобов планировщика — включить/отключить каждый джоб отдельно (состояние хранится в settings таблице)
• Редактируемое поле интервала прямо в таблице для collect_all, sq_*, pipeline_* джобов
• Live-синхронизация с APScheduler при работающем планировщике (джоб добавляется/удаляется без перезапуска)
• Disabled джобы остаются видны в таблице со статусом «отключён»

Test plan

• Открыть /scheduler, убедиться что таблица показывает колонки Вкл. / Джоб / Интервал / Следующий запуск
• Снять чекбокс — джоб пропадает из APScheduler (если запущен), статус меняется на «отключён»
• Поставить чекбокс — джоб возвращается, появляется next_run
• Остановить и запустить планировщик — disabled джобы не регистрируются
• Изменить интервал collect_all / sq_* — значение сохраняется в БД и применяется к APScheduler
• Все тесты: pytest tests/ -v -m "not aiosqlite_serial" -n auto

🤖 Generated with Claude Code

[axisrow]

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 3m 4s —— View job

---

Code Review

• Read changed files
• Analyze for critical issues
• Post review findings

---

Overall the implementation is solid. Found a few real bugs worth fixing and one logic gap.

---

Bug 1 — Silent no-op success for photo_due / photo_auto interval updates

src/web/routes/scheduler.py lines 205–232

_VALID_JOB_ID_RE matches photo_due and photo_auto, so POSTing to /scheduler/jobs/photo_due/set-interval passes validation, hits none of the if/elif branches, and returns msg=interval_updated — nothing was stored. The UI blocks this via interval_editable=False, but the endpoint is callable directly and misleads callers.

Fix: add an explicit guard before the redirect, or add photo_due/photo_auto to the regex only for the toggle endpoint.

Fix this →

---

Bug 2 — Missing pipeline existence check before update_generate_interval

src/web/routes/scheduler.py lines 228–232

The sq_* branch checks if sq: before updating. The pipeline branch does not:

elif job_id.startswith(("pipeline_run_", "content_generate_")):
    pid = int(pid_str)
    await db.repos.content_pipelines.update_generate_interval(pid, minutes)  # silent no-op if pid doesn't exist

If the pipeline was deleted between page load and form submit, the UPDATE silently updates 0 rows and the user gets a success message. Low probability, but easy to fix for consistency.

Fix this →

---

Bug 3 — sync_pipeline_jobs doesn't evict disabled-but-active jobs from a running scheduler

src/scheduler/manager.py lines 304–333

The cleanup loop removes jobs whose pipeline is inactive (not in active_ids). It does not remove jobs that are disabled via the scheduler_job_disabled:* setting:

for job in existing_jobs:
    if job.id.startswith("pipeline_run_") and job.id not in active_ids:
        self._scheduler.remove_job(job.id)   # only removes inactive pipelines
    # disabled but active pipeline_run_X stays here

In the add loop, disabled jobs are skipped — so they're neither removed nor rescheduled. This creates a state where a disabled pipeline job can remain live in APScheduler if sync_pipeline_jobs is called after a desync (e.g., interval update triggered while the job is somehow still registered). The toggle path calls remove_job directly so normal usage is safe, but a desynchronized state won't be recovered.

The same issue exists in sync_search_query_jobs (lines 271–288): cleanup only removes inactive queries, not disabled ones.

Fix this →

---

Minor — N+1 DB queries in _build_jobs_context

src/web/routes/scheduler.py lines 99–108

for j in jobs:
    val = await db.repos.settings.get_setting(f"scheduler_job_disabled:{j['job_id']}")

One query per job. With ~6–10 jobs this is fine, but it's worth noting. Could be batched into a single WHERE key IN (...) query if the settings repo grows to support it.

---

No security vulnerabilities found beyond the above. The job ID regex whitelist (_VALID_JOB_ID_RE) is correctly applied on both endpoints, interval clamping is correct, and the toggle logic is sound for the normal path.

[axisrow]

N+1 queries in _build_jobs_context — acknowledged, skipping. With 5–10 scheduler jobs the overhead is negligible (scheduler page auto-refreshes every 5s only when tasks are active). Batching would require a WHERE key IN (...) method on the settings repo — not worth the complexity for this use case.

Bugs 1–3 fixed in ea31ade:

• photo_due/photo_auto now return error=invalid_job from set-interval
• Pipeline existence check added before update_generate_interval
• sync_search_query_jobs and sync_pipeline_jobs cleanup loops now also evict jobs disabled via scheduler_job_disabled setting

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 1m 23s —— View job

---

Code Review

• Read changed files
• Analyze fixes from ea31ade
• Check for new critical issues
• Post review findings

---

All three bugs from the previous review are correctly fixed. One new issue surfaced from the interaction between Bug 1's fix and the existing update_interval code path.

---

Bug — update_interval throws unhandled JobLookupError when collect_all is disabled

src/scheduler/manager.py line 194 / src/web/routes/scheduler.py line 216

When collect_all is disabled, it is removed from APScheduler. If a user then POSTs to /scheduler/jobs/collect_all/set-interval, the route:

1. Saves the new interval to DB (line 215) ✓
2. Calls sched.update_interval(minutes) (line 216) — which calls reschedule_job on a job that no longer exists in APScheduler → APScheduler raises JobLookupError → the endpoint returns HTTP 500

The DB is left with the correct value (so the interval will be right when the job is re-enabled), but the caller gets a server error instead of the expected msg=interval_updated redirect. This is a user-visible regression introduced by the correct decision to remove disabled jobs from the scheduler.

Fix: in update_interval, catch JobLookupError (or check get_job(self._job_id) first) and skip the reschedule_job call when the job is absent — only update _current_interval_minutes. The DB write already happened in the route, so the new interval will be picked up on re-enable.

Fix this →

---

No other critical issues found. The three original fixes are correct:

• photo_due/photo_auto now correctly return error=invalid_job from set-interval
• Pipeline existence guard added before update_generate_interval
• Cleanup loops in sync_search_query_jobs and sync_pipeline_jobs now evict disabled-but-active jobs via is_job_enabled check

[axisrow]

Fixed in 03e7666: update_interval now checks get_job() before calling reschedule_job — if collect_all is disabled and absent from APScheduler, only _current_interval_minutes is updated (the new interval will be applied on re-enable). No JobLookupError possible.

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 1m 21s —— View job

---

Code Review

• Read changed files
• Analyze fix from 03e7666
• Check for new critical issues
• Post review findings

---

The fix in 03e7666 is correct. update_interval now unconditionally updates _current_interval_minutes first, then only calls reschedule_job if the APScheduler job actually exists — no JobLookupError possible. Critically, when the job is re-enabled via sync_job_state (line 73), it reads self._current_interval_minutes which is now always up-to-date regardless of whether the job was in the scheduler at the time of the interval change. The logic is sound end-to-end.

No new critical issues found. All four bugs across the review rounds have been fixed:

• photo_due/photo_auto correctly return error=invalid_job from set-interval
• Pipeline existence check guards update_generate_interval
• sync_search_query_jobs and sync_pipeline_jobs cleanup loops evict disabled-but-active jobs
• update_interval skips reschedule_job when collect_all is absent from APScheduler

One observation (not blocking): sync_job_state silently no-ops when re-enabling photo_due/photo_auto if self._task_enqueuer is None (lines 79–92 of manager.py). The DB setting is set to enabled but the APScheduler job is never added. This is correct behavior — if task_enqueuer isn't configured the jobs can't run — but it means the UI will show the job as enabled with no next_run indefinitely. Harmless in practice since task_enqueuer is always wired in production, but worth knowing.

PR is ready to merge.