fix(backups): keep schedule form edits and protect system jobs on reload
The schedule form effect read cronExpression, so choosing a preset or typing a
cron re-ran it and reset the form to the saved (disabled) schedule; Save then
persisted enabled=false. Seed the form under untrack() from the saved schedule only.
A runtime load_schedules() (PUT /environments/{id}/schedule) pruned every job
outside backup/translate/scenario schedules, deleting the recurring system ticks
registered by start(). Guard them with SYSTEM_JOB_IDS, kept in sync with start()
by a test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,25 @@ from __future__ import annotations
|
||||
from .scheduler_binding import facade
|
||||
from .config_manager import ConfigManager
|
||||
|
||||
# #region Core.SchedulerLifecycle.SystemJobIds [C:2] [TYPE Constant]
|
||||
# @BRIEF Recurring system jobs registered by start(); never owned by load_schedules().
|
||||
# @INVARIANT Every add_job id in SchedulerLifecycle.start() appears here, so the runtime
|
||||
# differential prune in load_schedules() cannot remove a system tick.
|
||||
SYSTEM_JOB_IDS = frozenset({
|
||||
"investigation_personal_delivery",
|
||||
"agent_lifecycle_retention",
|
||||
"scenario_retention_sweep",
|
||||
"scenario_cancel_drain_finalizer",
|
||||
"scenario_queued_dispatch",
|
||||
"scenario_revision_materialization",
|
||||
"mcp_approved_dispatch",
|
||||
"provider_operation_reconciliation",
|
||||
"authoring_exploration_dispatch",
|
||||
"verification_check_published_releases",
|
||||
"maintenance_auto_end",
|
||||
})
|
||||
# #endregion Core.SchedulerLifecycle.SystemJobIds
|
||||
|
||||
# #region Core.SchedulerLifecycle.Owner [C:3] [TYPE Class]
|
||||
# @BRIEF Start and stop the persistent scheduler and register recurring maintenance ticks.
|
||||
class SchedulerLifecycle:
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from .scheduler_binding import facade
|
||||
from .scheduler_lifecycle import SYSTEM_JOB_IDS
|
||||
|
||||
# #region Core.SchedulerReload.Owner [C:3] [TYPE Class]
|
||||
# @BRIEF Reload authoritative schedules and reconcile persisted scheduler jobs.
|
||||
@@ -13,6 +14,7 @@ class SchedulerReload:
|
||||
# @BRIEF Load backup and active translation schedules from config and DB, re-registering all jobs.
|
||||
# @PRE config_manager must have valid configuration; database is accessible.
|
||||
# @POST All enabled backup jobs and active translation schedules are re-registered in APScheduler (via persistent jobstore if configured).
|
||||
# @POST System jobs (SYSTEM_JOB_IDS) registered by start() are never removed, so a runtime reload keeps them.
|
||||
# @SIDE_EFFECT Removes existing jobs then syncs authoritative set from config + DB. The SQLAlchemyJobStore ensures durability of registered jobs across restarts.
|
||||
def load_schedules(self):
|
||||
with facade().belief_scope("SchedulerService.load_schedules"):
|
||||
@@ -126,10 +128,13 @@ class SchedulerReload:
|
||||
|
||||
# #region Core.Scheduler.Reload.prune_obsolete [C:3] [TYPE Function]
|
||||
# @BRIEF Remove only jobs absent from the reconstructed authoritative set.
|
||||
# @INVARIANT System jobs (SYSTEM_JOB_IDS) are not schedule-owned and are never pruned.
|
||||
def _prune_obsolete(self, desired_job_ids):
|
||||
try:
|
||||
current_jobs = self.scheduler.get_jobs()
|
||||
for job in current_jobs:
|
||||
if job.id in SYSTEM_JOB_IDS:
|
||||
continue
|
||||
if job.id not in desired_job_ids:
|
||||
try:
|
||||
self.scheduler.remove_job(job.id)
|
||||
|
||||
@@ -752,3 +752,38 @@ def test_load_schedules_prune_removes_obsolete_job(service, mock_scheduler):
|
||||
service.load_schedules()
|
||||
mock_scheduler.remove_job.assert_called_once_with("stale_job")
|
||||
# #endregion Test.Scheduler.TestLoadSchedulesPruneSuccess
|
||||
|
||||
|
||||
# #region Test.Scheduler.TestRuntimeReloadKeepsSystemJobs [C:2] [TYPE Function]
|
||||
# @BRIEF A runtime load_schedules() (PUT /environments/{id}/schedule) prunes only schedule-owned jobs.
|
||||
# @TEST_EDGE runtime_reload -> system ticks survive, disabled backup job removed, enabled one kept
|
||||
def test_load_schedules_runtime_reload_keeps_system_jobs(service, mock_scheduler):
|
||||
from src.core.scheduler_lifecycle import SYSTEM_JOB_IDS
|
||||
|
||||
config = _build_config_manager(
|
||||
environments=[
|
||||
_build_env("prod", "0 2 * * *", enabled=True),
|
||||
_build_env("dev", "0 3 * * *", enabled=False),
|
||||
]
|
||||
)
|
||||
existing = [SimpleNamespace(id=jid) for jid in sorted(SYSTEM_JOB_IDS)]
|
||||
existing += [SimpleNamespace(id="backup_prod"), SimpleNamespace(id="backup_dev")]
|
||||
mock_scheduler.get_jobs.return_value = existing
|
||||
with patch.object(service.config_manager, "get_config", return_value=config.get_config()):
|
||||
service.load_schedules()
|
||||
removed = {c.args[0] for c in mock_scheduler.remove_job.call_args_list}
|
||||
assert removed == {"backup_dev"}
|
||||
# #endregion Test.Scheduler.TestRuntimeReloadKeepsSystemJobs
|
||||
|
||||
|
||||
# #region Test.Scheduler.TestSystemJobIdsMatchStart [C:2] [TYPE Function]
|
||||
# @BRIEF SYSTEM_JOB_IDS lists exactly the jobs start() registers, so the prune guard cannot drift.
|
||||
def test_system_job_ids_match_start_registrations(service, mock_scheduler):
|
||||
from src.core.scheduler_lifecycle import SYSTEM_JOB_IDS
|
||||
|
||||
mock_scheduler.running = False
|
||||
with patch.object(service, "load_schedules"):
|
||||
service.start()
|
||||
registered = {c.kwargs["id"] for c in mock_scheduler.add_job.call_args_list}
|
||||
assert registered == SYSTEM_JOB_IDS
|
||||
# #endregion Test.Scheduler.TestSystemJobIdsMatchStart
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
@UX_STATE VerificationLoading -> The version's Verify button is busy.
|
||||
@UX_STATE VerificationResult -> Inline status chip + hashes under the version, announced via aria-live.
|
||||
@UX_FEEDBACK Toasts for trigger/schedule/delete outcomes; TaskDrawer auto-opens for started backups.
|
||||
@UX_STATE ScheduleEditing -> Form seeded from the saved schedule; switch/preset/cron edits persist until save or env change.
|
||||
@UX_RECOVERY Schedule save rolls back optimistic state on failure; Retry on load error.
|
||||
@UX_REACTIVITY LocalState -> $state(), Derived -> $derived(), env change -> $effect reload.
|
||||
@RATIONALE Flat file browser hid identity: copies with the same title looked like one dashboard.
|
||||
@@ -26,7 +27,7 @@
|
||||
@REJECTED Keeping FileList as the primary view — raw files remain reachable via "Open file storage".
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { onDestroy } from 'svelte';
|
||||
import { onDestroy, untrack } from 'svelte';
|
||||
import { t, locale } from '$lib/i18n/index.svelte.js';
|
||||
import { log } from '$lib/cot-logger';
|
||||
import { api, API_REQUEST_TIMEOUT } from '$lib/api';
|
||||
@@ -160,11 +161,18 @@
|
||||
loadCatalog();
|
||||
});
|
||||
|
||||
// Seed the schedule form from the saved schedule only when the environment or its
|
||||
// saved schedule changes; form edits (switch, preset, cron input) must not re-trigger it.
|
||||
$effect(() => {
|
||||
if (!selectedEnv) return;
|
||||
scheduleEnabled = selectedEnv.backup_schedule?.enabled ?? false;
|
||||
cronExpression = selectedEnv.backup_schedule?.cron_expression ?? '0 0 * * *';
|
||||
schedulePreset = presetForCron(cronExpression);
|
||||
const saved = selectedEnv.backup_schedule;
|
||||
const enabled = saved?.enabled ?? false;
|
||||
const cron = saved?.cron_expression ?? '0 0 * * *';
|
||||
untrack(() => {
|
||||
scheduleEnabled = enabled;
|
||||
cronExpression = cron;
|
||||
schedulePreset = presetForCron(cron);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Actions ─────────────────────────────────────────────────────
|
||||
|
||||
@@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => ({
|
||||
getBackupCatalog: vi.fn(),
|
||||
createTask: vi.fn(),
|
||||
getEnvironmentsList: vi.fn(),
|
||||
updateEnvironmentSchedule: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('$lib/i18n/index.svelte.js', () => ({
|
||||
@@ -20,7 +21,7 @@ vi.mock('$lib/i18n/index.svelte.js', () => ({
|
||||
vi.mock('$lib/cot-logger', () => ({ log: vi.fn() }));
|
||||
vi.mock('$lib/api', () => ({
|
||||
API_REQUEST_TIMEOUT: 1000,
|
||||
api: { getEnvironmentsList: mocks.getEnvironmentsList, createTask: mocks.createTask, updateEnvironmentSchedule: vi.fn() },
|
||||
api: { getEnvironmentsList: mocks.getEnvironmentsList, createTask: mocks.createTask, updateEnvironmentSchedule: mocks.updateEnvironmentSchedule },
|
||||
}));
|
||||
vi.mock('$lib/toasts.svelte.js', () => ({ notifications: { success: vi.fn(), error: vi.fn(), show: vi.fn() } }));
|
||||
vi.mock('$lib/stores/taskDrawer.svelte.js', () => ({ openDrawerForTaskIfPreferred: vi.fn() }));
|
||||
@@ -55,6 +56,7 @@ beforeEach(() => {
|
||||
const now = new Date().toISOString();
|
||||
mocks.getEnvironmentsList.mockResolvedValue([{ id: 'env-1', name: 'devta', backup_schedule: { enabled: true, cron_expression: '0 0 * * *' } }]);
|
||||
mocks.createTask.mockResolvedValue({ id: 'task-1' });
|
||||
mocks.updateEnvironmentSchedule.mockReset().mockResolvedValue({ message: 'ok' });
|
||||
mocks.getBackupCatalog.mockResolvedValue({
|
||||
environment: 'devta',
|
||||
dashboards: [
|
||||
@@ -105,4 +107,34 @@ describe('BackupManager catalog', () => {
|
||||
expect(mocks.getBackupCatalog).toHaveBeenLastCalledWith('env-2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('BackupManager schedule form', () => {
|
||||
const openForm = async () => {
|
||||
mocks.getEnvironmentsList.mockResolvedValue([
|
||||
{ id: 'env-1', name: 'devta', backup_schedule: { enabled: false, cron_expression: '0 0 * * *' } },
|
||||
]);
|
||||
render(BackupManager);
|
||||
await fireEvent.click(await screen.findByText(ruStorage.catalog.schedule_configure));
|
||||
await fireEvent.click(screen.getByRole('switch'));
|
||||
};
|
||||
|
||||
it('keeps the enabled switch when a preset is chosen and saves it', async () => {
|
||||
await openForm();
|
||||
// Ui.Select ids collide across instances, so pick the preset select by position, not label.
|
||||
const preset = screen.getAllByRole('combobox')[1] as HTMLSelectElement;
|
||||
await fireEvent.change(preset, { target: { value: 'weekly' } });
|
||||
expect(screen.getByRole('switch').getAttribute('aria-checked')).toBe('true');
|
||||
await fireEvent.click(screen.getByText(ruCommon.save));
|
||||
expect(mocks.updateEnvironmentSchedule).toHaveBeenCalledWith('env-1', { enabled: true, cron_expression: '0 0 * * 1' });
|
||||
});
|
||||
|
||||
it('keeps a manually typed cron expression and saves it', async () => {
|
||||
await openForm();
|
||||
const cron = screen.getByLabelText(ruTasks.cron_label) as HTMLInputElement;
|
||||
await fireEvent.input(cron, { target: { value: '30 2 * * *' } });
|
||||
expect(cron.value).toBe('30 2 * * *');
|
||||
await fireEvent.click(screen.getByText(ruCommon.save));
|
||||
expect(mocks.updateEnvironmentSchedule).toHaveBeenCalledWith('env-1', { enabled: true, cron_expression: '30 2 * * *' });
|
||||
});
|
||||
});
|
||||
// #endregion Test.Backups.BackupManager
|
||||
|
||||
Reference in New Issue
Block a user