Deactivate one-shot Claude jobs after firing

[dcellison]

Summary

One-shot Claude jobs (schedule_type == "once", job_type == "claude") are never deactivated in the database after they fire. The reminder path correctly handles this, but the Claude job path does not.

APScheduler's run_once removes the job from the in-memory scheduler queue (so it won't fire again during the current process), but the database row remains active=1. On restart, _register_new_jobs catches it as an "expired one-shot" (lines 108-113) and deactivates it then, but between firing and the next restart, the stale row pollutes /api/jobs listings and could theoretically re-fire if timing is unlucky.

Root Cause

Reminder path (correct) at cron.py:251-257:

        # One-shot reminders auto-deactivate after firing.
        if data["schedule_type"] == "once":
            await sessions.deactivate_job(job_id)
        return

Claude job path (missing) at cron.py:342-353:

        else:
            # Non-conditional or non-auto-remove: always deliver the response
            msg = f"[Job: {data['name']}]\n{response_text}"
            try:
                log_message(direction="assistant", chat_id=chat_id, text=msg)
                await context.bot.send_message(chat_id=chat_id, text=msg)
            except Forbidden:
                log.warning("Job %d: chat %d is gone, deactivating", job_id, chat_id)
                await sessions.deactivate_job(job_id)
                job.schedule_removal()
            except Exception:
                log.exception("Failed to send job %d result", job_id)

The function ends with no schedule_type == "once" check. The same gap exists in the CONDITION_NOT_MET path (lines 317-340).

Affected Code Paths

Three Claude job delivery paths need the one-shot deactivation check:

1. CONDITION_MET path (lines 299-314) - already calls deactivate_job via auto_remove logic, but only if auto_remove is set. A non-auto-remove one-shot Claude job taking this path (not possible currently, but defensive) would not deactivate.
2. CONDITION_NOT_MET path (lines 317-340) - no deactivation for one-shot
3. Non-conditional else path (lines 342-353) - no deactivation for one-shot

Fix

Add the same pattern used by the reminder path after each Claude job delivery path:

if data["schedule_type"] == "once":
    await sessions.deactivate_job(job_id)

This should go after the successful send_message call in each path, mirroring the reminder logic at line 255.

Severity

High - database inconsistency; one-shot Claude jobs remain active forever in job listings until the next restart.

[dcellison]

Triage by Kai

Triage summary: One-shot Claude jobs (schedule_type == 'once') are never deactivated in the database after firing because the three Claude job delivery paths in cron.py lack the schedule_type == 'once' deactivation check that the reminder path correctly implements. The fix is well-scoped: add await sessions.deactivate_job(job_id) after each Claude delivery path, mirroring the existing reminder logic.

Priority: high
Labels applied: good first issue

[dcellison]

@wangwangbobo Thanks for picking this up! We've implemented a fix in PR #119 based on Kai's spec.

A few notes on your branch:

1. Base commit - Your branch includes reversions of the triage prompt hardening (Harden triage prompt against injection via randomized boundaries #110), cron message chunking (Split long cron job responses to respect Telegram message limit #111), and the test history contamination fix (Test suite contaminates production history files #118). These are unrelated to the one-shot fix and would revert security and reliability work if merged.

2. Placement - The deactivation check at line 348 is outside the elif/else block structure, so it runs after all three branches. The CONDITION_MET path would deactivate twice (once via auto_remove, once via your check). The spec places the check inside each branch individually.

3. Missing path - The CONDITION_NOT_MET branch (path 2 in the spec) doesn't get the deactivation check. A one-shot auto-remove job whose condition isn't met would stay active in the DB.

4. Redundant guard - data.get("job_type") == "claude" isn't needed since we're already past the if job_type == "reminder": ... return block at line 269.

We appreciate the contribution! If you'd like to tackle another issue from the code review list, there will be several more opening up soon.