A full-suite owner_test cancels a pending requeue and defers the next heartbeat by a full interval

Summary

Publishing an authoritative compliance run defers that agent's next automatic heartbeat by a full check_interval_hours and clears requeued_at. There is no triggered_by predicate, so an owner-triggered full-suite test does it too.

Two consequences:

A requeue is cancelled by the owner's own next test. requeueForHeartbeat exists so an owner can get an automatic authoritative run. Any full-suite owner_test published afterwards undoes it.
Frequent owner testing starves the automatic check indefinitely. Every authoritative run pushes next_compliance_check_at forward. Test more often than check_interval_hours and the heartbeat never becomes due. There is no floor and nothing surfaces it.

Mechanism

server/src/db/compliance-db.ts:937-942, inside recordComplianceRun's status upsert:

WITH schedule_next AS (
  INSERT INTO agent_registry_metadata (agent_url, next_compliance_check_at)
  VALUES ($1, NOW() + INTERVAL '12 hours')
  ON CONFLICT (agent_url) DO UPDATE SET
    next_compliance_check_at = NOW() + make_interval(hours => agent_registry_metadata.check_interval_hours),
    requeued_at = NULL
)

Non-authoritative runs return before this (:847-852, "audit evidence cannot change the public card, storyboard materialization, scheduling, or transition notifications"), so the gate is is_authoritative, not triggered_by. Per server/src/addie/services/compliance-testing.ts:1480, a full-suite owner test is authoritative:

is_authoritative: result.completeness !== 'timed_out' && result.overall_status !== 'auth_required' && !storyboardIds?.length,

A scoped owner test carries storyboardIds and is not authoritative, so it correctly leaves scheduling alone. It is specifically the unscoped "Recheck & retest" that defers.

Eligibility is gated at :1552:

AND (m.next_compliance_check_at IS NULL OR m.next_compliance_check_at < NOW())

and requeueForHeartbeat (:1609-1613) clears exactly that gate:

ON CONFLICT (agent_url) DO UPDATE SET next_compliance_check_at = NULL, requeued_at = NOW()

So the ordering added for #7632 (ORDER BY requeued_at ASC NULLS LAST, … at :1561) only ranks agents that are already eligible. Re-arming the gate removes the agent from the candidate set entirely, before ordering applies.

Repro

Requeue an agent. next_compliance_check_at = NULL, requeued_at = NOW(). Agent is eligible and sorts first.
Before the batch reaches it, click "Recheck & retest" (unscoped).
On publish: next_compliance_check_at = NOW() + 12h, requeued_at = NULL.
Agent is ineligible for 12h. The requeue is gone, with no indication it was dropped.

check_interval_hours defaults to 12 (migrations/378_agent_monitoring_controls.sql, CHECK BETWEEN 6 AND 168).

Why I do not think this is intended dedupe

Skipping an automatic run right after a full-suite run is defensible on freshness grounds. These two behaviours are not:

It defeats the feature two recent changes were built to deliver. Owners cannot obtain an authoritative compliance run: human refresh fenced with no ETA, and requeue cannot reach agents with a recent last_checked_at (heartbeat orders by last_checked_at ASC) #7632 and fix(registry): serve explicit heartbeat requeues first and reference the refresh fence tracking issue #7648 exist so an owner can obtain an automatic authoritative run; this silently reverses a requeue issued seconds earlier.
The codebase already guards this exact invariant one function away. deferComplianceCheckAfterInconclusiveTarget (:1443-1452) carries the comment "A manual requeue (NULL) must win over deferral" and enforces it with AND next_compliance_check_at > NOW(). The schedule_next CTE has no equivalent guard. The same rule is protected in one path and violated in the other.

There is also a monitoring-independence angle. A heartbeat is the registry's own check; an owner test is owner-initiated and runs under owner-chosen conditions. If owner activity displaces the independent check indefinitely, the card still looks fresh via last_checked_at while nothing independent has run. For a surface that issues badges, those are not equivalent.

Observed effect

This is not theoretical, and it skewed the sample in #7355. Both agents at the centre of that issue are among the most owner-tested in the registry and show the oldest heartbeats. I own both and I generated that owner_test traffic, so I am part of the reported symptom there. Filing this separately because it is self-inflicted and distinct from the throughput question in #7355 — see #7355 (comment) for the fleet data.

Suggested fix

Do not re-arm the gate from a run the owner asked for, and never silently drop a pending requeue:

add a triggered_by = 'heartbeat' predicate to the schedule_next CTE, or
mirror the existing guard so a pending requeue (next_compliance_check_at IS NULL / requeued_at IS NOT NULL) survives publication.

If deferring on owner_test is deliberate, a floor would still be worth having — force eligibility once last_checked_at from a heartbeat exceeds some multiple of the interval, so the independent check cannot be starved indefinitely.

I have not verified production state; everything above is read from main and from the public registry API.

@EvgenyAndroid

Triage

Classification: Bug
Bucket(s): compliance-suite, admin-tool
Status: ready-to-implement
Milestone: Evergreen

What the experts said:

code-reviewer (re-run after @EvgenyAndroid's challenge): Both Fix 2 variants are definitively broken. Variant A (requeued_at IS NULL gate): line 942 is the only place requeued_at is ever cleared — gating the DO UPDATE on requeued_at IS NULL means the heartbeat that serves the requeue also skips the clear, leaving the agent permanently eligible and consuming a batch slot forever. Variant B (next_compliance_check_at > NOW() guard): eligibility at :1552 is IS NULL OR < NOW(), so at publish time > NOW() is always false — the guard suppresses every DO UPDATE on every heartbeat and the entire fleet goes permanently due. The deferComplianceCheckAfterInconclusiveTarget guard works only because it runs mid-flight before publishing, where the value can still be future — the precondition does not transfer. triggered_by != 'owner_test' is the correct predicate; use != 'owner_test' (not = 'heartbeat') so manual and webhook runs still advance the cadence.
internal-tools-strategist (prior, still stands): Monitoring-independence angle is legitimate — last_checked_at looks fresh from owner testing alone with no independent heartbeat behind it. P1, not P0 (12h natural workaround exists).

My take: @EvgenyAndroid's challenge is correct throughout. The prior recommendation of Fix 2 is wrong on both variants. The correct fix is the triggered_by predicate approach from the original issue, refined to != 'owner_test' to preserve cadence advancement for manual and webhook runs — those are infrastructure-side actors, not agent owners displacing monitoring.

Implementation scope (updated):

File: server/src/db/compliance-db.ts:937-942 — the schedule_next CTE in recordComplianceRun.
Fix: Add input.triggered_by ?? 'heartbeat' as $7 in the parameter array (currently only 6 parameters: :992-999). In the ON CONFLICT DO UPDATE branch, append WHERE $7 != 'owner_test' so only non-owner-org runs advance next_compliance_check_at and clear requeued_at.
Edge case: The INSERT branch (new-row path for a first-ever run) fires unconditionally regardless of triggered_by; a first-run owner_test still creates a 12h lookahead. Low impact — corrects on the next heartbeat — but worth a follow-up.
Test matrix (5 cases):
1. requeue → heartbeat publishes → next_compliance_check_at = NOW() + interval, requeued_at = NULL (requeue consumed)
2. requeue → unscoped owner_test publishes → next_compliance_check_at IS NULL, requeued_at IS NOT NULL (requeue survives — the primary regression case)
3. requeue → manual/webhook publishes → schedule advances, requeue clears (validates != 'owner_test' boundary)
4. no requeue → heartbeat publishes → cadence advances normally
5. no requeue → unscoped owner_test publishes → next_compliance_check_at unchanged (monitoring independence)
No changeset: server-only, no protocol surface affected.

Triaged by Claude Code. Session: https://claude.ai/code/session_01Q4u5SgFaDRX5zYNHihnVjb

Generated by Claude Code

Hold on this one before it gets coded. Both forms of Fix 2 break, and I think I caused the confusion by citing deferComplianceCheckAfterInconclusiveTarget as a precedent for the invariant without saying its guard expression does not transplant.

Fix 2 as scoped — gate the DO UPDATE on requeued_at IS NULL — makes a requeue permanent.

:942 is the only place requeued_at is ever cleared. I grepped the whole file; the only other writes are the setter at :1611-1613 and reads at :1544/:1561. So if the DO UPDATE is skipped whenever requeued_at IS NOT NULL, the requeue is never cleared by anything — including the heartbeat that serves it. That agent then sits at next_compliance_check_at = NULL (always eligible per :1552) with a non-null requeued_at (always first under ORDER BY requeued_at ASC NULLS LAST), and gets re-tested in every batch forever. It also permanently consumes one of the 10 batch slots.

Fix 2 as "mirror the existing WHERE next_compliance_check_at > NOW() guard" never fires.

Selection at :1552 is:

AND (m.next_compliance_check_at IS NULL OR m.next_compliance_check_at < NOW())

A heartbeat only ever runs on an agent whose next_compliance_check_at is NULL or already past. At the moment it publishes, next_compliance_check_at > NOW() is false by construction. The guard would suppress the update on every heartbeat, so the cadence would never advance and the whole fleet would go permanently due. That is a worse outage than the one in #7355.

The guard works in deferComplianceCheckAfterInconclusiveTarget because that path runs without publishing a verdict — it releases a lock mid-flight, where the value can still be in the future. Same invariant, different precondition. My citing it as precedent was about intent, and I should have been explicit that the expression itself does not port.

The triggered_by gate is the one that holds. Advance next_compliance_check_at and clear requeued_at only on triggered_by = 'heartbeat'; leave both untouched otherwise. A served requeue is still cleared by the heartbeat that served it, so nothing becomes permanent, and an owner_test stops re-arming the gate.

On the code-reviewer's objection that this "would prevent owner tests from ever refreshing the schedule when no requeue is pending" — that is the intended outcome, not a side effect. It is the same monitoring-independence point the internal-tools-strategist raised: last_checked_at can look fresh from owner testing alone with no independent heartbeat behind it. Letting an owner test advance the independent cadence is the behaviour that produces that, so preserving it would preserve the defect.

Suggested test matrix, replacing the one in the triage:

requeue → unscoped owner_test publishes → next_compliance_check_at IS NULL, requeued_at IS NOT NULL (requeue survives)
requeue → heartbeat publishes → next_compliance_check_at = NOW() + interval, requeued_at IS NULL (requeue consumed — this is the case Fix 2 breaks)
no requeue → heartbeat publishes → cadence advances normally
no requeue → unscoped owner_test publishes → next_compliance_check_at unchanged
scoped owner_test (storyboardIds non-empty) → non-authoritative, returns at :847-852, scheduling untouched

fix(compliance): keep owner tests from re-arming the heartbeat schedule

@EvgenyAndroid

Fix is up in #7691.

What it does.

recordComplianceRun's schedule_next CTE now takes triggered_by as $7 (defaulting to 'heartbeat', same as the runs insert) and the ON CONFLICT DO UPDATE is gated with WHERE $7 != 'owner_test'.

An unscoped owner test still publishes the verdict, card, and badges, but no longer touches next_compliance_check_at or requeued_at.

Neither Fix 2 variant was used, for the reasons laid out above.

How this compares to @EvgenyAndroid's proposed fix.

It's the same fix in all but one respect:

Same location and same approach: gate the schedule update on triggered_by, nothing else in the CTE changes.
Same invariants preserved: the heartbeat that serves a requeue still clears it, so a requeue can't become permanent or hold a batch slot forever, and the fleet cadence still advances.
Same stance on the code-reviewer's objection: an owner test with no requeue pending does not refresh the schedule. That's pinned by a test.
Your five-case matrix is implemented as written, including the scoped-owner-test case.

The one difference is the predicate. You proposed = 'heartbeat'; the PR uses != 'owner_test', following the triage refinement.

The two are identical for owner_test and heartbeat and only diverge for manual (admin support runs) and webhook: under the PR those advance the cadence and consume a requeue, like a heartbeat; under yours they'd leave the schedule alone.

I went with != 'owner_test' because the issue's framing is about the owner displacing independent checks, which admin-side runs don't do, and because it avoids a redundant re-test an hour after an admin run.

But yours is the stricter, simpler rule ("only the scheduler moves the schedule"), and switching is a one-token change with one test assertion flipping. Happy to go either way if you have a preference.

One thing beyond the comment.

The INSERT branch is handled here rather than left as a follow-up. A first-ever owner_test on an agent with no metadata row now inserts next_compliance_check_at = NULL instead of NOW() + 12h.

That agent was already due before the owner test (no row ⇒ NULL in the eligibility join), so this keeps it as due as it was: the same rule applied to the insert path.

Tests.

Your five cases, plus:

unlabelled run defaults to heartbeat scheduling
an already-overdue agent stays due after an owner test
first-run owner_test → immediately eligible
first-run heartbeat → default 12h cadence
manual / webhook → cadence advances

With the fix reverted, the four owner-test cases fail; the rest pin unchanged behaviour.

Not in this PR.

The floor (force eligibility once no heartbeat has run in N × check_interval_hours) and surfacing "last independent check" on the card.

The floor has read-side traps: an agent deferred via deferComplianceCheckAfterInconclusiveTarget records no run, so a naive floor would re-select it every hour and eat batch slots, and it would have to distinguish the in-flight lock from a real deferral. Worth its own issue if there's appetite.

test(compliance): register adcontextprotocol#7680 fixture agents on t…

Closing. The fix in #7969 works on the live fleet.

Before/after evidence for sell.nofluffadvisory.com: a heartbeat at 2026-10-10 09:44, an owner_test at 17:42, then the next heartbeat at 22:10, staying on the agent's ~12h cadence. Before the fix, the owner_test would have pushed that heartbeat to about 05:42 the next day.

The broader fleet recovery came from #7844 (Oct 1), not #7969. The remaining throughput and per-agent gap questions are tracked in #7355.

Navigation Menu

Repository navigation

A full-suite owner_test cancels a pending requeue and defers the next heartbeat by a full interval #7680

Summary

Mechanism

Repro

Why I do not think this is intended dedupe

Observed effect

Suggested fix

Triage

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

A full-suite owner_test cancels a pending requeue and defers the next heartbeat by a full interval #7680

Description

Summary

Mechanism

Repro

Why I do not think this is intended dedupe

Observed effect

Suggested fix

Activity

bokelley commented on Sep 24, 2026

Triage

EvgenyAndroid commented on Sep 24, 2026

therevoltingx commented on Sep 25, 2026

bokelley commented on Oct 13, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions