fix(infra): a zero graph TTL is stale even when the clock reads the mtime back (#692) - #950

Claude finished @cdeust's task in 1m 32s —— View job

PR #950 review: done

• Read the diff and CLAUDE.md
• Ran five reviews: code quality, documentation accuracy, performance, security and test coverage
• Posted the review: fix(infra): a zero graph TTL is stale even when the clock reads the mtime back (#692) #950 (comment)

There are no Blockers. The zero-TTL early return in graph_is_stale is correct, and the regression test fails on every platform without the fix. The posted review has 3 Notes, none needing action before merge.

I could not run pytest, ruff or check_craftsmanship.py because the sandbox did not allow them, so those checks rely on CI.

Verdict: approve

Review 1 of 3 (ADR-1119): PR #950

I ran five reviews on this PR: code quality, documentation accuracy, performance, security and test coverage. None of them found a Blocker.

• The fix is correct. mcp_server/infrastructure/pipeline_graph_ttl.py:54 returns stale when the TTL is 0 or less, before it reads the file's age. claim_reanalysis_slot already does the same at :75, so a zero TTL now behaves the same in both functions. Its result no longer depends on how coarse the clock is.
• The regression test catches the bug on every platform. test_ttl_zero_is_stale_at_age_zero freezes the clock at the file's mtime, so the age is exactly 0.0. Without the fix, 0.0 > 0.0 is False and the test fails on Linux too, not only on Windows.
• The boundary test is stable. For an mtime near 1.7e9, mtime + 3600.0 is computed exactly and subtracting the mtime gives back exactly 3600. The age is therefore exactly 1.0 hour on every platform.
• Rules from CLAUDE.md: no layer changes and no new constants, and the changed method is 15 lines.
• Not run by me: this sandbox would not let me run pytest, ruff or check_craftsmanship.py. The PR's own test plan and CI cover those checks.

Notes, no action before merge

• tests_py/infrastructure/test_pipeline_graph_ttl.py:61: the old comment says a just-written file is older than 0 hours "by a tiny amount". This PR shows that is false on a coarse clock, so it should say a zero TTL is stale by itself.
• pipeline_graph_ttl.py:44 and the module docstring at :8 still describe stale as "missing or older than the TTL". Both could add "or the TTL is zero".
• test_a_graph_exactly_ttl_old_is_not_stale passes with or without the fix. It pins the strict > comparison (it kills the mutmut survivors) and does not guard Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692 itself.

Verdict: approve

Merci pour la revue 1. Je ne pousse pas pour les trois notes, car elles sont non bloquantes :

• Le commentaire de test_ttl_zero_everything_stale (« older by a tiny amount ») et les docstrings de graph_is_stale et du module (« missing or older than the TTL ») sont corrigés au prochain push de code sur cette PR, s'il y en a un. Ils ne justifient pas à eux seuls de relancer la CI et la revue.
• C'est voulu que test_a_graph_exactly_ttl_old_is_not_stale passe avec ou sans le correctif : il épingle la comparaison stricte > et le diviseur (les deux survivants mutmut). test_ttl_zero_is_stale_at_age_zero garde Windows: full-suite debt of 194 failing tests (measured 2026-10-10), fix order by root cause #692.

Generated by Claude Code