From ad509282082d43b0d6af8df0fad42fe91029dcd7 Mon Sep 17 00:00:00 2001 From: nftpoetrist Date: Fri, 8 May 2026 23:24:19 +0300 Subject: [PATCH] fix(cron): clean up output dir when auto-removing repeat-limited job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mark_job_run() pops a job from jobs.json when its repeat limit is reached (lines 737-741) but did not delete OUTPUT_DIR/. save_job_output() creates that directory on every run, so completed one-shot jobs leave one orphaned output directory per job. remove_job() gained the identical cleanup in PR #21882 — this extends the fix to the auto-delete path inside mark_job_run(), which is the only other code path that removes a job. Fix: add the same OUTPUT_DIR/ rmtree guard used by remove_job(), scoped to the repeat-limit branch. No behavior change for recurring jobs or jobs without a repeat limit. Fixes #22065 --- cron/jobs.py | 5 +++++ tests/cron/test_jobs.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/cron/jobs.py b/cron/jobs.py index 566db1e6dbc1..c8f485c6ee8e 100644 --- a/cron/jobs.py +++ b/cron/jobs.py @@ -738,6 +738,11 @@ def mark_job_run(job_id: str, success: bool, error: Optional[str] = None, # Remove the job (limit reached) jobs.pop(i) save_jobs(jobs) + # Mirror remove_job()'s cleanup — output dirs from the + # completed runs would otherwise accumulate unnoticed. + job_output_dir = OUTPUT_DIR / job_id + if job_output_dir.exists(): + shutil.rmtree(job_output_dir) return # Compute next run diff --git a/tests/cron/test_jobs.py b/tests/cron/test_jobs.py index 0405f997b143..a5a82426564a 100644 --- a/tests/cron/test_jobs.py +++ b/tests/cron/test_jobs.py @@ -452,6 +452,39 @@ def test_oneshot_still_completes_when_next_run_is_none(self, tmp_cron_dir): assert updated["enabled"] is False assert updated["state"] == "completed" + def test_repeat_limit_cleans_up_output_dir(self, tmp_cron_dir): + """mark_job_run() must delete OUTPUT_DIR/ when auto-removing a + repeat-limited job, matching what remove_job() does (PR #21882 parity). + Without this, completed one-shot jobs leave orphaned output directories. + """ + job = create_job(prompt="Once", schedule="30m", repeat=1) + job_id = job["id"] + + # Simulate save_job_output() having written at least one output file. + output_dir = tmp_cron_dir / "cron" / "output" / job_id + output_dir.mkdir(parents=True) + (output_dir / "2026-05-08_12-00-00.md").write_text("output") + + assert output_dir.exists() + + mark_job_run(job_id, success=True) + + assert get_job(job_id) is None, "job was not removed after repeat limit" + assert not output_dir.exists(), "output dir was not cleaned up after auto-delete" + + def test_repeat_limit_no_output_dir_is_safe(self, tmp_cron_dir): + """mark_job_run() auto-delete is safe when no output was ever saved + (OUTPUT_DIR/ does not exist) — the cleanup must be a no-op. + """ + job = create_job(prompt="Once", schedule="30m", repeat=1) + output_dir = tmp_cron_dir / "cron" / "output" / job["id"] + + assert not output_dir.exists() + + mark_job_run(job["id"], success=True) + + assert get_job(job["id"]) is None + class TestAdvanceNextRun: """Tests for advance_next_run() — crash-safety for recurring jobs."""