Skip to content

fix(workflow-runs): take part in the job TTL purge so the headless API doesn't leak - #288

Open
kevin9327 wants to merge 1 commit into
lightningpixel:devfrom
kevin9327:fix/workflow-runs-job-purge
Open

fix(workflow-runs): take part in the job TTL purge so the headless API doesn't leak#288
kevin9327 wants to merge 1 commit into
lightningpixel:devfrom
kevin9327:fix/workflow-runs-job-purge

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What

The headless /workflow-runs API leaks job records. It shares the _jobs, _cancel_events and _completed_at dicts with the /generate endpoints, but never took part in their TTL purge:

  • create_run_from_image never called _purge_old_jobs(). /generate/from-image purges terminal jobs older than _JOB_TTL on every call; /workflow-runs/from-image didn't, so records only ever got swept if a /generate call happened to run the purge. For a pure automation client (the surface /workflow-runs exists for), nothing triggers it and _jobs grows without bound.
  • cancel_run never stamped _completed_at. _purge_old_jobs() only sweeps entries that have a completion timestamp, and cancel_job (the /generate sibling) sets one when it cancels. cancel_run set status="cancelled" but no timestamp, so a cancelled run could never be purged — a permanent leak of one JobStatus + one threading.Event per cancellation.

Fix

Mirror the /generate endpoints: call _purge_old_jobs() when a run is created, and record _completed_at[run_id] when a run is cancelled. Collection routing is intentionally left untouched.

# create_run_from_image
_purge_old_jobs()
_jobs[job_id] = JobStatus(...)

# cancel_run
if job.status in ("pending", "running"):
    job.status = "cancelled"
    _completed_at[run_id] = time.monotonic()

Verification

  • Added api/tests/test_workflow_runs_lifecycle.py: one test proves a stale terminal job is evicted when a new run is created, the other proves a cancelled run gets a completion stamp so the purge can reach it. Both fail before, pass after.
  • python -m unittest discover -s tests in api/ (venv with fastapi + python-multipart + httpx): all pass, 0 failures.

…I doesn't leak

The /workflow-runs surface shares _jobs, _cancel_events and _completed_at with
the /generate endpoints, but never participated in their TTL purge:

- create_run_from_image never called _purge_old_jobs(), so terminal job records
  accumulated indefinitely unless a /generate/from-image call happened to sweep
  them. That is the opposite of the intended use — /workflow-runs is the
  headless automation surface, where nothing else triggers the purge.
- cancel_run set status="cancelled" but never stamped _completed_at, so
  _purge_old_jobs() (which only sweeps entries that have a completion time) could
  never evict a cancelled run — a permanent leak of a JobStatus + threading.Event
  per cancellation.

Mirror what cancel_job and generate_from_image already do: purge on create, and
record the completion time on cancel. Collection routing is intentionally left
untouched here.

Adds api/tests/test_workflow_runs_lifecycle.py (both cases fail before, pass after).

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant