fix(#4718): harden parse-cache key with ctime_ns + preserve epoch-0 mtime fallback
Dual-gate (Codex + Opus) findings on the Claude Code transcript parse cache: 1. Cache-key staleness hardening: add st_ctime_ns to the (path, mtime_ns, size, max_messages) key so a rare same-size, same-mtime in-place edit can't serve a stale parse. Size is already the strong discriminator for append-only JSONL; ctime_ns is cheap defense-in-depth (a spurious bump costs one harmless re-parse, never a stale read). 2. Timestamp-fallback bug-for-bug fix: the row-builder guard used identity (first_ts is None and last_ts is None) but the field assignment uses truthiness (first_ts or last_ts or _mtime). They disagreed on a falsy 0.0 timestamp (epoch-0/1970 transcripts), leaving created_at/updated_at/last_message_at = None instead of the file mtime the pre-cache inline code fell back to. Guard now uses the same 'not first_ts and not last_ts' test. Adds 3 regression tests: epoch-0 mtime fallback, the shared-dict read-only contract (pins the load-bearing invariant), and same-size+mtime ctime invalidation.
This commit is contained in:
1 parent
fdb734a6a0
commit
d61ca37bd1
2 files changed
+113
-7
No files matched your search
@@ -147,3 +147,98 @@ def test_get_claude_code_sessions_warm_uses_cache(tmp_path, monkeypatch):
|
||||
assert cold_calls == 3 # parsed each file once on the cold build
|
||||
assert calls["n"] == cold_calls # warm build added zero re-parses
|
||||
assert [s["title"] for s in cold] == [s["title"] for s in warm]
|
||||
|
||||
|
||||
def test_epoch_zero_timestamps_fall_back_to_mtime(tmp_path):
|
||||
"""A transcript whose timestamps parse to 0.0 still gets a real mtime fallback.
|
||||
|
||||
The cached row-builder guards the mtime fallback with ``not first_ts and not
|
||||
last_ts`` so a falsy-but-not-None ``0.0`` timestamp (epoch-0 / 1970
|
||||
transcript) falls back to the file mtime, matching the pre-cache inline
|
||||
``first_ts or last_ts or path.stat().st_mtime``. An identity (``is None``)
|
||||
guard would have left these rows with ``None``.
|
||||
"""
|
||||
import api.models as models
|
||||
|
||||
models.clear_claude_code_parse_cache()
|
||||
projects_dir = tmp_path / "claude" / "projects"
|
||||
fixture = projects_dir / "p" / "s.jsonl"
|
||||
# All message timestamps are epoch 0 -> _parse_claude_code_timestamp -> 0.0.
|
||||
rows = [
|
||||
{"summary": "Epoch QA"},
|
||||
{"timestamp": "1970-01-01T00:00:00Z", "message": {"role": "user", "content": "hi"}},
|
||||
{"timestamp": "1970-01-01T00:00:00Z", "message": {"role": "assistant", "content": "ok"}},
|
||||
]
|
||||
_write_jsonl(fixture, rows)
|
||||
|
||||
sessions = models.get_claude_code_sessions(projects_dir=projects_dir)
|
||||
assert len(sessions) == 1
|
||||
s = sessions[0]
|
||||
# Must NOT be None — falls back to the file mtime (a real positive float).
|
||||
assert s["created_at"] is not None
|
||||
assert s["updated_at"] is not None
|
||||
assert s["last_message_at"] is not None
|
||||
assert s["created_at"] > 0
|
||||
|
||||
|
||||
def test_parse_cache_dicts_are_read_only_contract(tmp_path):
|
||||
"""Pin the load-bearing invariant: per-message dicts are SHARED across hits.
|
||||
|
||||
The cache returns a shallow ``list(messages)`` copy, so the per-message dicts
|
||||
are shared between calls. Every production caller treats them as read-only;
|
||||
this test documents that contract by proving the sharing exists — a future
|
||||
caller that mutates a returned dict in place would corrupt the cache, and
|
||||
this test makes that sharing explicit so such a change is a conscious one.
|
||||
"""
|
||||
import api.models as models
|
||||
|
||||
models.clear_claude_code_parse_cache()
|
||||
fixture = tmp_path / "claude" / "projects" / "p" / "s.jsonl"
|
||||
_write_jsonl(fixture, _rows())
|
||||
|
||||
first_msgs, *_ = models._parse_claude_code_jsonl_cached(fixture)
|
||||
second_msgs, *_ = models._parse_claude_code_jsonl_cached(fixture)
|
||||
# List wrappers are distinct copies (append isolation, covered above)...
|
||||
assert first_msgs is not second_msgs
|
||||
# ...but the dict objects are shared (the read-only contract). If a future
|
||||
# change deep-copies on read, update this assertion deliberately.
|
||||
assert first_msgs[0] is second_msgs[0]
|
||||
|
||||
|
||||
def test_parse_cache_invalidates_on_same_size_mtime_ctime_edit(tmp_path, monkeypatch):
|
||||
"""A same-size, same-mtime in-place edit still misses the cache via ctime_ns."""
|
||||
import os
|
||||
import api.models as models
|
||||
|
||||
models.clear_claude_code_parse_cache()
|
||||
fixture = tmp_path / "claude" / "projects" / "p" / "s.jsonl"
|
||||
_write_jsonl(fixture, _rows("aaaaa"))
|
||||
|
||||
calls = {"n": 0}
|
||||
real = models._parse_claude_code_jsonl
|
||||
|
||||
def _counting(path, **kw):
|
||||
calls["n"] += 1
|
||||
return real(path, **kw)
|
||||
|
||||
monkeypatch.setattr(models, "_parse_claude_code_jsonl", _counting)
|
||||
|
||||
first = models._parse_claude_code_jsonl_cached(fixture)
|
||||
assert first[0][0]["content"] == "aaaaa"
|
||||
st = fixture.stat()
|
||||
|
||||
# Rewrite with same byte length and force the SAME mtime back; the os.utime
|
||||
# call still stamps ctime with the current wall clock, so a real in-place
|
||||
# edit moves ctime even when size+mtime are unchanged. Sleep so that ctime is
|
||||
# measurably distinct from the original mtime_ns we restore.
|
||||
time.sleep(0.02)
|
||||
_write_jsonl(fixture, _rows("bbbbb"))
|
||||
os.utime(fixture, ns=(st.st_atime_ns, st.st_mtime_ns))
|
||||
st2 = fixture.stat()
|
||||
assert st2.st_size == st.st_size
|
||||
assert st2.st_mtime_ns == st.st_mtime_ns
|
||||
assert st2.st_ctime_ns != st.st_ctime_ns # only ctime moved (the edit signal)
|
||||
|
||||
second = models._parse_claude_code_jsonl_cached(fixture)
|
||||
assert calls["n"] == 2 # re-parsed despite identical size+mtime (ctime caught it)
|
||||
assert second[0][0]["content"] == "bbbbb"
|
||||
Reference in new issue
Block a user