fix: preserve original expiration window on stale claim reclaim

[jolovicdev]

Summary

Preserves the original expires_at value when reclaiming a stale running claim, rather than recalculating it from the current time. This prevents cache entries from living longer than intended when a worker crash or slowdown causes a claim to be reclaimed.

Changes

• src/cashet/async_executor.py — Removed the expires_at recalculation on stale claim reclaim. The original expiration window is now kept intact.
• src/cashet/models.py — Added clarifying comments on the expires_at field to document the stable-horizon design intent.
• tests/test_store.py — Added test_reclaimed_stale_claim_keeps_original_expires_at covering sync stale reclaim.
• tests/test_async_client.py — Added test_cached_task_with_ttl_and_stale_reclaim covering async stale reclaim.

Rationale

When a claim is reclaimed after a worker dies, the previous behavior recalculated expires_at = now + ttl. This effectively extends the cache lifetime by the full TTL from the reclaim moment, even though the task may have already been running for a while. By preserving the original expires_at, callers get a predictable expiration horizon tied to the original submission time.

The task_def.ttl field is still propagated to the reclaimed claim so that downstream inspection reflects the current submission's preferences, but the actual eviction timestamp remains stable.

Testing

All 296 existing tests pass (45 skipped for Redis). The two new tests verify that stale claim reclamation works correctly for both sync and async paths with cache=True.

[ds-review]

PR Review

PR: fix: preserve original expiration window on stale claim reclaim

Important

Verdict: Request changes — tests must assert that expires_at is preserved after reclaim to prevent regression.

Findings

Pri	Location	Finding	Action
P2	tests/test_store.py:325	Missing assertion that expires_at is preserved after reclaim	Add assertion comparing commit's expires_at to original value
P2	tests/test_async_client.py:312	Missing assertion that expires_at is unchanged and cache hit works	Add expires_at assertion and second submission to confirm cache hit
P3	tests/test_async_client.py:282	Test name includes "with_ttl" but no TTL is set	Rename test to reflect actual scenario

Notes

• Both new tests validate that stale claims are reclaimed and function re-executes, but neither checks that the original expires_at is preserved. Without these assertions, a regression that recalculates expires_at would not be caught.
• The async test also does not verify that a subsequent submission returns from cache without re-execution.

What To Do Next

• Add assertions for expires_at in both test_reclaimed_stale_claim_keeps_original_expires_at and test_cached_task_with_ttl_and_stale_reclaim.
• In the async test, add a second submit call to confirm the cache hit (counter should not increment).

[ds-review]

P2 This test does not assert that expires_at is preserved after reclaim. Without it, a regression that recalculates expires_at would not be caught. Add an assertion comparing the commit's expires_at to the original value. For example:

Suggested change

	assert log[0].status.value == "completed"
	assert log[0].expires_at is None

[ds-review]

[tests/test_async_client.py:312]

P2 The test verifies the function runs but does not check that the reclaimed commit's expires_at is unchanged, nor that a second submission returns the cached result without re-executing. Add an assertion on expires_at and a second submit call to confirm the cache hit.

        # Retrieve commit after reclaim
        log = await async_client.log()
        assert len(log) > 0
        assert log[0].expires_at is None  # original expires_at preserved

        # Second submission should return cached result and not increment counter
        ref2 = await async_client.submit(work, _cache=True)
        assert await ref2.load() == 1
        assert counter == 1

[ds-review]

[tests/test_async_client.py:282]

P3 The test name includes "with_ttl" but the task definition has no TTL set, making the name potentially misleading. Either rename the test to reflect the actual scenario, or add a _ttl parameter to cover the TTL behaviour.

    async def test_cached_task_and_stale_reclaim(
        self, async_client: AsyncClient
    ) -> None:

[jolovicdev]

Closing in favor of a more comprehensive fix.