fix: cascade-delete TaskRun records when removing a ScheduledTask

[soumojit-D48]

Fixes #2668

Describe the changes you have made in this PR -

remove_task() was only deleting the ScheduledTask from the JSON store but leaving orphaned TaskRun records in the SQLite claim store. Added delete_runs() to claim_store.py and wired it into remove_task() so run history is cleaned up when a task is deleted.

Changes:

• app/scheduler/claim_store.py — added delete_runs(task_id, db_path) that deletes all TaskRun rows for a given task
• app/scheduler/store.py — remove_task() now calls delete_runs() after removing the task from JSON; best-effort, logs warning on failure, return value unchanged
• tests/scheduler/test_claim_store.py — 4 new tests for delete_runs (scoped delete, idempotent, empty DB, multi-run)
• tests/scheduler/test_store.py — 2 new tests for cascade (deletes runs, doesn't affect other tasks)

Demo/Screenshot for feature changes and bug fixes -

$ pytest tests/scheduler/test_claim_store.py tests/scheduler/test_store.py -v
======================= 27 passed in 1.38s ========================

---

Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

• Yes, I used AI assistance (continue below)

If you used AI assistance:

• I have reviewed every single line of the AI-generated code
• I can explain the purpose and logic of each function/component I added
• I have tested edge cases and understand how the code handles them
• I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:

The TaskRun records live in a separate SQLite DB (scheduler.db) with a task_id reference to the parent ScheduledTask. When the parent is deleted, those records become inaccessible orphans.

Two changes:

1. delete_runs() — a focused SQL DELETE scoped by task_id. Returns rowcount, no-ops if DB doesn't exist, idempotent.

2. remove_task() — derives the SQLite path from the JSON store's directory and calls delete_runs() after the JSON write succeeds. Wrapped in try/except so a DB failure doesn't revert the JSON deletion — orphaned runs are acceptable (waste space) but a stuck task is not.

---

Checklist before requesting a review

• I have added proper PR title and linked to the issue
• I have performed a self-review of my code
• I can explain the purpose of every function, class, and logic block I added
• I understand why my changes work and have tested them thoroughly
• I have considered potential edge cases and how my code handles them
• If it is a core feature, I have added thorough tests
• My code follows the project's style guidelines and conventions

---

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

[github-actions]

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

[greptile-apps]

Greptile Summary

This PR fixes orphaned TaskRun records that were left in the SQLite claim store after a ScheduledTask was deleted from the JSON store. The cascade is intentionally best-effort — a SQLite failure logs a warning but does not revert the JSON deletion.

• claim_store.py: adds delete_runs(task_id, db_path) — a focused DELETE scoped by task_id, safe when the DB or table is absent, idempotent.
• store.py: remove_task() derives the DB path using _DB_FILENAME (fixing the previously hardcoded string) and calls delete_runs() outside the file lock, wrapped in try/except.
• Tests: 4 unit tests for delete_runs and 2 integration tests for the cascade in remove_task.

Confidence Score: 5/5

Safe to merge — the cascade delete is correctly scoped, best-effort, and covered by integration tests.

The implementation is straightforward: delete_runs correctly guards against a missing DB, is idempotent, and uses parameterised SQL. The remove_task integration holds the file lock only for the JSON write, then performs the SQLite cleanup outside it, which is the right call given the best-effort contract. The only notable gap is the test helper hardcoding the DB filename rather than using the shared constant, which is a minor maintainability nit and does not affect correctness today.

No files require special attention; tests/scheduler/test_store.py has a minor constant-reuse nit but is otherwise correct.

Important Files Changed

Filename	Overview
app/scheduler/claim_store.py	Adds delete_runs() to cascade-delete all TaskRun rows for a task_id; correct early exit when DB is absent, proper schema ensure, commit, and rowcount return.
app/scheduler/store.py	Wires delete_runs() into remove_task() after JSON write; uses _DB_FILENAME constant to derive the DB path, best-effort with try/except and warning log.
tests/scheduler/test_claim_store.py	Four new unit tests for delete_runs: scoped delete, idempotency, empty DB, and multi-run; all well-structured.
tests/scheduler/test_store.py	Two new integration tests for cascade; helper _db_path() hardcodes "scheduler.db" rather than importing _DB_FILENAME, creating potential silent test drift if the constant is renamed.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant store.py as store.remove_task()
    participant JSON as JSON Store (FileLock)
    participant claim_store.py as claim_store.delete_runs()
    participant SQLite as SQLite (scheduler.db)

    Caller->>store.py: remove_task(task_id)
    store.py->>JSON: acquire lock → filter out task → _save_raw()
    JSON-->>store.py: written (lock released)
    store.py->>claim_store.py: delete_runs(task_id, db_path)
    claim_store.py->>SQLite: path.exists()?
    alt DB exists
        SQLite-->>claim_store.py: True
        claim_store.py->>SQLite: "DELETE FROM task_runs WHERE task_id = ?"
        SQLite-->>claim_store.py: rowcount
        claim_store.py-->>store.py: deleted N rows
        store.py->>store.py: "log.info (if N > 0)"
    else DB absent
        SQLite-->>claim_store.py: False
        claim_store.py-->>store.py: 0 (no-op)
    end
    store.py-->>Caller: True

Loading

_{Reviews (2): Last reviewed commit: "fix: use shared DB filename constant, mo..." | Re-trigger Greptile}

[soumojit-D48]

@greptile-apps review again

[soumojit-D48]

@muddlebee Hi pls check this PP and let me know. Thank u!!