Repository navigation
next_compliance_check_at conflates cadence, in-flight lock and skip deferral, which blocks any independent-check floor #7693
Description
Split out of #7680 at @therevoltingx's suggestion (see #7691). His objection to a naive floor is correct, and tracing why points at something more basic: next_compliance_check_at encodes three different states, and nothing can tell them apart.
The overload
One column carries all of:
- Normal cadence —
recordComplianceRun'sschedule_nextCTE setsNOW() + check_interval_hourson publish (compliance-db.ts:937-942). - In-flight batch lock —
compliance-heartbeat.ts:145-153setsNOW() + lockSecondsfor all selected agents at batch start, before any of them runs:Atconst perAgentBudgetMs = HOSTED_FULL_COMPLIANCE_TIMEOUT_MS + (2 * HOSTED_TARGET_DISCOVERY_TIMEOUT_MS); const lockSeconds = urls.length * (perAgentBudgetMs / 1000) + 300;
limit: 10that is10 × 660 + 300 = 6900s, ~1h55m, applied viaunnestto every url in the batch. A batch that dies after two agents leaves the other eight locked for the remainder. - Skip deferral —
deferComplianceCheckAfterInconclusiveTargetadds anothercheck_interval_hoursand records no run. It is called from eight distinct paths incompliance-heartbeat.ts(:159,:216,:234,:269,:400,:417,:507,:519).
Eligibility is only next_compliance_check_at IS NULL OR < NOW() (compliance-db.ts:1552). So "running right now", "skipped eight times in a row", and "checked normally an hour ago" are indistinguishable, internally and externally.
Why that blocks a floor
A floor keyed on the column re-selects locked agents mid-batch and eats slots, which is exactly the trap raised on #7691. A floor keyed on "no run recorded recently" cannot see the skip paths at all, because they deliberately record nothing. There is no third signal to key on.
Why it is worth fixing
Nothing currently bounds how long an agent can go without an independent check. Fleet telemetry from 2026-09-23T17:07Z: 33 of 34 agents exceeded the declared 12-hour cadence, median staleness 8.4 days, with a bimodal distribution — 27 agents at 2–14 days, nothing at all between 14 and 60 days, then 2 never checked and 3 at 71–78 days. That void is the signature of two different failure modes, and neither leaves a public trace. Related throughput collapse in #7355.
#7691 closes the owner-initiated starvation path, which is real progress. The eight skip paths and the pre-emptive batch lock are untouched by it.
Suggested shape
Separate the lock from the schedule before adding any floor:
- give the in-flight lock its own column (
compliance_lock_untilor similar) so eligibility can exclude locked agents without consulting the cadence; - record the skip reason and attempt time (
last_heartbeat_attempt_at+ reason) even when no run is published, so repeated deferral is visible rather than silent.
The floor then becomes expressible and safe: force eligibility when time since the last successful heartbeat exceeds N × check_interval_hours and the agent is not currently locked. It also makes the deferral loop diagnosable, which today it is not.
Dependency worth noting
From outside, none of this is verifiable — requeued_at and next_compliance_check_at are not exposed on the compliance card, so an owner cannot distinguish "queued", "locked", "deferred" or "forgotten". I have that filed platform-side as AAO #616. A floor without that surface would be correct but unobservable, so they are worth sequencing together.
Filed as a separate issue rather than on #7680 because the fix is structural and the two should not block each other.
Activity
bokelley commentedon Sep 25, 2026
Triage
Classification: Bug
Bucket(s): compliance suite, admin / ops tools
Status: ready-to-implement
Milestone: Evergreen
What the experts said:
ADD COLUMN IF NOT EXISTSpatterns; floor guard must usecompliance_lock_until < NOW(), not IS NULL, to handle mid-batch crashes that leave the column populated until TTL expiry.My take: Both experts converge with no blockers. No protocol surface is touched — this is server/DB infrastructure only, no changeset needed. PR #7691 addresses the orthogonal owner_test starvation path and can proceed independently.
Implementation scope:
compliance_lock_until TIMESTAMPTZ,last_heartbeat_attempt_at TIMESTAMPTZ,last_heartbeat_skip_reason TEXT, andlast_heartbeat_success_at TIMESTAMPTZtoagent_registry_metadata(nullable, no backfill)compliance-heartbeat.ts:145–153) to writecompliance_lock_untilinstead ofnext_compliance_check_at; update all 8deferComplianceCheckAfterInconclusiveTargetcall sites (:159,:216,:234,:269,:400,:417,:507,:519) to write attempt time + reason tolast_heartbeat_attempt_at/last_heartbeat_skip_reason; writelast_heartbeat_success_at = NOW()on authoritative publish inrecordComplianceRun(compliance-db.ts:937–942)compliance-db.ts:1552): excludecompliance_lock_until >= NOW(), add floor keyed onlast_heartbeat_success_at— a skip refresheslast_heartbeat_attempt_at, so the floor must key on confirmed success to rescue skip-starved agents; derive fromagent_compliance_runsonly as a fallback if the stored column is absentTriaged by Claude Code. Session: https://claude.ai/code/session_01C327NZtnnsSrcfRRjLPw6c
Generated by Claude Code
EvgenyAndroid commentedon Sep 25, 2026
Scope looks right, and the code-reviewer's lock-guard refinement is better than what I wrote.
compliance_lock_until < NOW()rather thanIS NULLpreserves the self-healing property the current design already has —compliance-heartbeat.ts:139notes "a mid-loop crash re-queues the agent after this TTL", and anIS NULLguard would strand a crashed batch's agents until something cleared the column by hand. Worth keeping that comment's intent attached to the new column.One thing to settle before implementation: the floor cannot key on
last_heartbeat_attempt_at.The scope has the 8 skip sites writing attempt time + reason, and the floor keyed on that same column. But a skip is an attempt, so an agent that is selected and skipped every cycle keeps refreshing
last_heartbeat_attempt_atand never trips the floor. That is precisely the starved agent the floor exists to rescue — the 71–78-day outliers are the population most likely to be failing in a skip path rather than never being selected.The floor wants the last successful heartbeat: either a new
last_heartbeat_success_atwritten only on an authoritative heartbeat publish, or derived fromagent_compliance_runs(newest row withtriggered_by = 'heartbeat' AND is_authoritative AND NOT dry_run). A stored column is cheaper in the eligibility query and avoids a correlated subquery per candidate.last_heartbeat_attempt_atandlast_heartbeat_skip_reasonare still worth adding — they make the deferral loop diagnosable, which is the other half of the problem. They just answer "did we try" rather than "did it work", and only the second one can drive the floor.So: three columns as scoped, plus the success timestamp, and the floor keyed on the success timestamp with
compliance_lock_until < NOW()as the guard.On sequencing against AAO #616 — the strategist is right and I was wrong to put it first. Fixing the data should not wait on a platform-side visibility ticket; #616 becomes more useful afterwards, since it would then have something truthful to surface. Withdrawing that dependency.
EvgenyAndroid commentedon Sep 25, 2026
Concretely, so the scope above can be picked up as written — two amendments:
Migration: add a fourth column,
last_heartbeat_success_at TIMESTAMPTZ, alongside the three listed.Eligibility (
compliance-db.ts:1552): key the floor onlast_heartbeat_success_at, notlast_heartbeat_attempt_at. Guard stayscompliance_lock_until < NOW() OR IS NULLper the code-reviewer's note.The write site is convenient: #7691 is already threading
triggered_byinto theschedule_nextCTE as$7, solast_heartbeat_success_at = NOW()can be set in that sameON CONFLICT DO UPDATEunder$7 = 'heartbeat', with no new call site and no extra query. The two changes compose rather than conflict — #7691 supplies the parameter this needs.Restating the failure in one line, since it is the whole reason for the swap: the 8 skip sites write
last_heartbeat_attempt_at, so an agent that is selected and skipped every cycle refreshes that column forever and never crosses a floor keyed on it.last_heartbeat_attempt_atandlast_heartbeat_skip_reasonstill earn their place — they answer "did we try and why did it not work", which is what makes the deferral loop diagnosable. They just cannot drive the floor.Synaeve commentedon Nov 12, 2026
#8954 splits the heartbeat batch lock (and the #8188 crash quarantine and contention hold) into
compliance_lock_until, records every heartbeat attempt's outcome inlast_heartbeat_attempt_at/last_heartbeat_skip_reason/last_heartbeat_success_at, and makes eligibility check the lock separately from the cadence. Migration 635 moves in-flight claims and backfills attempt/success history from heartbeat runs.It does not add the independent-check floor, so it only refs this issue. As specified (success older than N × interval, lock free), the floor would re-select a failing agent on every 5-minute tick, because every deferral clears the lock. Since #7844 and #8188 already cap skip and crash backoff at 48 h, a success-keyed floor would mostly retry failing agents faster than that backoff. Could maintainers decide N, its precedence over that backoff, and the throttle a safe floor needs (for example
last_heartbeat_attempt_at < NOW() - X)? With the backfilled success column it is then a single predicate.EvgenyAndroid commentedon Nov 12, 2026
Thanks @Synaeve. #8954 is the split this issue asked for, and the backfill from heartbeat runs means the new columns start with real history.
You're right about the floor as I specified it. Keyed on success with only the lock guard, it bypasses the cadence: an agent whose attempts end inconclusive comes back on every tick, and the 48 h backoff from #7844 stops applying (the #8188 quarantine lives in the lock, so the guard still holds that one). I'd withdraw it as an eligibility rule.
What's left after #8954 is ordering. The cadence gate and the lock now decide correctly who is due. But
getAgentsDueForCheckranks the regular cadence bylast_checked_at, and every published run sets that, owner tests included. #7969 took owner tests out of the heartbeat schedule and #7648 put requeues first; the sort key is the one place owner tests still steer the heartbeat. When more agents are due than a batch takes, an agent whose owner tests often keeps sorting to the back of its tier and can go without an independent check while eligible the whole time. Forcing eligibility can't help an agent that is already eligible.So instead of a floor, carry
m.last_heartbeat_success_atintodue_agentsand sort on it wherelast_checked_atsits now:That makes N, the precedence question and the throttle unnecessary. Eligibility still requires the cadence and a free lock, so a failing agent is never picked before its backoff expires, and within its tier the agent with the oldest independent check is served first once it is due.
eligible_backlogalready shows whether this matters in production: if it stays at or below the batch limit, ordering starves no one. Either way, I think #7693 can close once #8954 and this sort-key change are in.Synaeve commentedon Nov 13, 2026
Thanks, that is a better fix than the floor. #8954 now ranks due agents within each tier by
last_heartbeat_success_at ASC NULLS FIRST, exactly as you proposed. Only thelast_checked_atterm changed: eligibility still needs the cadence and a free lock (commit 15667ce, with an integration test where a frequently owner-tested agent with an older heartbeat success is served first). The PR now closes #7693.