fix: prune deleted rollouts from terminal completion log

[elren (elrensmin)]

Description

delete_rollout (agentlightning/server/routes/rollouts.py:216) pops the rollout from _rollouts and _events but never removes its id from _terminal_order. The append-only completion log backing cursor pagination.

Consequences:

• GET /api/rollouts/terminal reports total_terminal/next_after for rollouts that no longer exist.
• Re-enqueueing a deleted client-assigned rollout_id and driving it terminal again appends a duplicate entry.
• Breaks the documented invariant next_after == total_terminal when caught up (asserted at tests/server/test_endpoints.py:395).

Fix (root cause, not symptom)

In delete_rollout, also prune the completion log:

if rollout_id in _terminal_order:
    _terminal_order.remove(rollout_id)

Removing from the list shifts subsequent indices, which is exactly what keeps the cursor consistent. This also fixes the re-enqueue case: after delete prunes the id, re-completion appends exactly once.

Tests

Added two regression tests in tests/server/test_endpoints.py:

• test_delete_removes_completed_rollout_from_terminal_log: delete a completed rollout, then total_terminal/next_after drop to 1 and only the surviving id is returned.
• test_delete_and_reenqueue_completion_is_logged_once: delete, re-enqueue with the same id, re-complete → logged exactly once.

Verification

• uv run pytest → 51 passed, 3 skipped (no regressions; new tests included).
• uv run ruff check agentlightning/server/routes/rollouts.py tests/server/test_endpoints.py → only 2 pre-existing E501 (line-too-long) at test_endpoints.py:363-364 in the untouched _terminal helper; my additions are clean.
• uv run ruff format --check on the two changed files → my additions are format-clean; the only remaining diff is pre-existing style drift in the _terminal helper and cursor test.

Note on pre-existing repo-wide failures (out of scope, not introduced here)

The repo does not currently pass its own lint/format gates cleanly. These are pre-existing and unrelated to this change; I deliberately left them untouched to keep the diff minimal:

• uv run ruff check . → 601 errors across the repo, dominated by W293 (whitespace, 411), E501 (line-too-long, 99), I001 (import sorting, 20), F541 (f-string without placeholders, 17), F401 (unused imports, 16), plus E402, F841, E001, W291, W292, F100, etc.
• uv run ruff format --check . → 51 files would be reformatted (44 already clean).
• uv run pyright → not installed in the dev toolchain (pyright is not a dev dependency in pyproject.toml); uvx pyright only reports reportMissingImports for fastapi/pydantic/httpx because of its isolated env, not real type errors.
  There is no CONTRIBUTING doc, no .pre-commit-config.yaml, and no CI lint/test workflow in this repo (only docs + skills deploy workflows), so these are the only gates available.

[elren (elrensmin)]

@microsoft-github-policy-service agree

[Copilot]

Pull request overview

This PR fixes cursor-pagination correctness for the “terminal rollouts” endpoint by ensuring that deleting a rollout also removes its ID from the in-memory completion log used to paginate terminal rollouts.

Changes:

• Update delete_rollout to prune deleted rollout IDs from _terminal_order.
• Add regression tests covering deletion effects on /api/rollouts/terminal totals/cursors and the delete→re-enqueue→complete flow.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
agentlightning/server/routes/rollouts.py	Prunes deleted rollout IDs from the terminal completion log to keep cursor pagination consistent.
tests/server/test_endpoints.py	Adds regression tests to prevent reintroduction of stale/duplicate terminal-log entries after delete and re-enqueue.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Sylvester Kaczmarek (sylvesterkaczmarek)]

This fixes stale IDs in the current snapshot, but compacting _terminal_order breaks already-issued positional cursors. For example, if a client consumes [a, b] and receives next_after=2, then a is deleted from [a, b, c], the list becomes [b, c] and the next request with after=2 skips c. Because the endpoint documents this as an append-only completion log, deletion needs tombstones/a monotonic sequence (or a cursor redesign), plus a regression where deletion happens between pages.