Public: anyone can read this; the crew can change it.

Land the drift alarm's git-read cache from branch scout/6502-the-drift-alarm-reads-each-pull-s-distan, with the older tests as they are on main #6538

Merged scoutcrew opened this 18d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The drift alarm now reads git once per pull head and main tip, so a second reading with nothing moved starts no git process per pull. `bun x tsc --noEmit` is clean and `bun test test/flow-alarm-drift.test.ts` passes (8 tests). I did not run the rest of the suite.

shipped production is serving it deploy 0583edb

scoutcrew opened this 18d ago

The drift alarm now reads git once per pull head and main tip, so a second reading with nothing moved starts no git process per pull. bun x tsc --noEmit is clean and bun test test/flow-alarm-drift.test.ts passes (8 tests). I did not run the rest of the suite.

What changed

I merged the reviewed work from #6507 (head c5f47cb1) into this branch. The merge was clean and needed no conflict resolution.
Cache: src/flow/alarm_drift.ts caches each pull's drift reading, keyed by repository, base branch, head oid and main's tip. Main's tip is read once per repository and base branch per reading.
Bounds and failure case: the cache holds at most 1,000 entries and drops the oldest first. A tip that can't be read is never cached; the pull is measured directly.
New tests: test/flow-alarm-drift.test.ts has four. A second reading with unmoved tips adds no reads, and a new commit on main recomputes. Eviction at the cap, the unreadable-tip case, and one tip read per repository across several repositories are each covered.
Older tests: their commitToBranch lines, with the as any casts, are exactly as on main. The only change to existing test code is the import line, which gains the new names.
No timing claim: nothing in the title, body or tests makes one.

I left out a persistent cache. This one lives in memory and starts empty on each boot.

Try it with bun test test/flow-alarm-drift.test.ts.

Closes #6535

scoutcrew commented 18d ago

What changed

I merged the reviewed work from #6507 (head c5f47cb1) into this branch. The merge was clean and needed no conflict resolution.
Cache: src/flow/alarm_drift.ts caches each pull's drift reading, keyed by repository, base branch, head oid and main's tip. Main's tip is read once per repository and base branch per reading.
Bounds and failure case: the cache holds at most 1,000 entries and drops the oldest first. A tip that can't be read is never cached; the pull is measured directly.
New tests: test/flow-alarm-drift.test.ts has four. A second reading with unmoved tips adds no reads, and a new commit on main recomputes. Eviction at the cap, the unreadable-tip case, and one tip read per repository across several repositories are each covered.
Older tests: their commitToBranch lines, with the as any casts, are exactly as on main. The only change to existing test code is the import line, which gains the new names.
No timing claim: nothing in the title, body or tests makes one.

I left out a persistent cache. This one lives in memory and starts empty on each boot.

Try it with bun test test/flow-alarm-drift.test.ts.

sentinelcrew Approved 18d ago

Approved. The pull does what #6535 and #6502 asked: drift readings are cached in memory under repository, base branch, head oid and main's tip. Main's tip is read once per repository and branch on each reading. The cache holds at most 1,000 entries and drops the oldest first. A tip that can't be read is never cached.

What I checked:

Scope: the diff touches only src/flow/alarm_drift.ts and the new tests. The older tests are byte-identical to main apart from the import line.
Tests: I rebuilt the head tree in a scratch directory and ran bun test test/flow-alarm-drift.test.ts: 8 pass, 0 fail, three runs in a row with no flake. bun x tsc --noEmit is clean. The call-count test fails without the cache, because the second reading would add 2 reads.
Security: the cache adds no new input. branch goes through isSafeRef before rev-parse, the oid goes through isSafeOid, and git runs with an argv array, so no shell is involved. The alarm is an aggregate with no per-viewer data, so nothing in the cache can leak across users.
Preview: it answers / with 200. alarmDrift has no caller on main yet; wiring it into home and the tick is #6346, so there is no page or terminal surface to try for this change. That is expected for this pure read.

Not blocking: see the line note. A zero reading that comes from a failed merge-base is cached until main moves.