[therevoltingx]

Closes #7680

Problem

recordComplianceRun re-armed the heartbeat schedule on every authoritative publish, regardless of who triggered the run. A full-suite ("Recheck & retest") owner_test therefore:

1. Cancelled a pending requeue. It set next_compliance_check_at = NOW() + interval and cleared requeued_at, removing the agent from the due set before the 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 ordering could apply. Nothing surfaced that the requeue was dropped.
2. Could starve the independent heartbeat indefinitely. Owners testing more often than check_interval_hours kept pushing the heartbeat out, while last_checked_at still looked fresh.

Fix

Gate the schedule_next CTE on triggered_by (passed as $7, defaulting to 'heartbeat' to match the runs insert):

• owner_test, existing row: DO UPDATE ... WHERE $7 != 'owner_test' leaves next_compliance_check_at and requeued_at untouched.
• owner_test, first-ever run: the metadata row is inserted with next_compliance_check_at = NULL, so the agent is immediately eligible for an independent heartbeat instead of waiting 12h. This covers the INSERT-branch follow-up noted in triage.
• heartbeat / manual / webhook: unchanged. The cadence advances and requeues are consumed. The heartbeat that serves a requeue still clears it, so a requeue can never become permanent.

Owner-test verdicts still publish to the public card, last_checked_at, and badges as before. Only scheduling changes.

This follows the != 'owner_test' predicate from the triage discussion. Both "Fix 2" variants (gate on requeued_at IS NULL, or mirror next_compliance_check_at > NOW()) were ruled out there because they break requeue consumption or the whole cadence.

Tests

10 new cases in server/tests/integration/compliance-run-publication.test.ts:

• requeue → full owner_test → requeue survives and the agent stays due; verdict still published
• requeue → heartbeat → requeue consumed, next check about 12h out
• requeue → manual / webhook → cadence advances, requeue cleared
• unlabelled run → treated as heartbeat
• scheduled future check → owner_test → not pushed further out
• overdue agent → owner_test → still due
• no metadata row → owner_test → row created with NULL (immediately eligible)
• no metadata row → heartbeat → default 12h cadence
• scoped (non-authoritative) owner_test → scheduling untouched

With the fix reverted, the 4 owner-test cases fail; the other 6 pin unchanged behavior. Also ran the compliance-heartbeat and compliance-db-last-write-wins unit suites (41/41) and tsc with no errors.

No changeset: server-only, no protocol surface.

🤖 Generated with Claude Code

Disposition (2026-10-09): superseded by merged #7969 (a4003bd7ac6b1889047ab828156af09dddf41f92). Its owner-test publication path preserves heartbeat scheduling, requeues and streaks, and initializes owner-only records without a heartbeat schedule. The existing replacement passed all 24 real PostgreSQL regressions, full normal local checks and all 56 remote checks with fresh final-head approval. The fallback is therefore no longer needed. This source branch is preserved; #7355 remains open for separate fleet/operator verification.

[aao-ipr-bot]

IPR Policy Agreement Required

@therevoltingx — thanks for the contribution. Before this PR can be merged, the AgenticAdvertising.Org IPR Policy requires your agreement.

To agree, post a new comment on this PR with the exact phrase:

I have read the IPR Policy

Your signature is recorded once and covers all contributions to AAO repositories. See signatures/README.md for what gets recorded and why.

[aao-secretariat]

Ladon verdict: Approve

Approve — server-only fix for #7680 with no blocking or medium findings.

What I checked:

• Scope is server/** only (compliance-db.ts + integration test) — no protocol surface touched (static/schemas/source/**, docs/reference/**, task defs, dist artifacts). No changeset required; correct no-changeset call.
• The fix gates the schedule_next CTE on triggered_by so owner_test runs publish the verdict but never re-arm the heartbeat: no-op on existing rows, NULL-and-immediately-due on first insert. Heartbeat/manual/webhook paths unchanged.
• NULL-safe predicate verified; no starvation or permanent-skip window.
• 10 tests pin every branch of the new conditional.

Gates: No no-auto-approve team match. gated_paths: false. high_risk: false. review_decision is REVIEW_REQUIRED, but row 2 only fires when gated_paths is true — it is not, so no hard gate applies. No critical/high/medium findings (rows 1, 4–8 do not fire). Falls through to row 9.

[therevoltingx]

I have read the IPR Policy

[aao-ipr-bot]

IPR Policy — signed

Thanks, @therevoltingx. Your agreement to the IPR Policy is recorded at signatures/ipr-signatures.json and applies to all AAO repositories.

[EvgenyAndroid]

I'd go with = 'heartbeat', and the reason is narrower than a style preference: triggered_by has no enum behind it.

It is a bare string at compliance-db.ts:1294 and z.string() at registry.ts:1511, and at registry-api.ts:8730 the value is passed straight through from the request. So != 'owner_test' is fail-open on an open value space. Anything that is not exactly that literal — a future trigger type, a new caller, a casing slip — silently acquires the power to advance the cadence and consume a pending requeue, without anyone touching this line or reviewing that decision. = 'heartbeat' fails closed: a value nobody has thought about does nothing to the schedule.

The two failure shapes are not symmetric either. A wrong != match destroys a requeue with no trace, and per #616 there is currently no owner-visible queue state at all, so it cannot be observed from outside. A wrong = match leaves an agent due, which shows up as an agent being re-tested — noisy, but visible.

The divergence set is smaller than the comparison suggests. webhook is not a triggered_by value anywhere: zero occurrences in registry-api.ts, zero in code search. (Worth checking directly rather than via code search, which does not index registry-api.ts — that has bitten me on this repo.) So the only real divergence is manual, and the documented contract at registry-api.ts:4246 is just the two owner/admin labels. manual is 4 runs out of 2,832 fleet-wide, and registry-api.ts:9921 already calls it "the legacy 'manual' label".

On the admin case specifically, I think it cuts the other way from the triage. An admin support run consuming an owner's pending requeue is the same defect shape as the one this issue is about: an action the owner did not initiate silently cancelling their explicit request. The owner-displacement framing is right, and an admin support run is not the registry's independent scheduled check either. Against that, the cost you are avoiding is one redundant heartbeat after an admin run, on 4 runs historically.

One correction to the switching cost: it is two places, not one. The INSERT branch is keyed on the same literal:

VALUES ($1, CASE WHEN $7 = 'owner_test' THEN NULL ELSE NOW() + INTERVAL '12 hours' END)
...
WHERE $7 != 'owner_test'

Those are consistent as written. Flip only the WHERE and manual gets NOW() + 12h on first run but no update on every subsequent one — first-run and steady-state disagree. Both need to become heartbeat-keyed together:

VALUES ($1, CASE WHEN $7 = 'heartbeat' THEN NOW() + INTERVAL '12 hours' ELSE NULL END)
...
WHERE $7 = 'heartbeat'

That said, this is a preference, not a blocker. The practical difference today is 4 runs, and your reasoning for the admin carve-out is coherent. If you keep !=, please leave a comment on the line saying the value space is an unvalidated string, so the next person adding a trigger type knows they are opting into scheduling power by default.

The INSERT branch was the right call to handle here. I left it out of the issue and I was wrong to — the reasoning holds: no metadata row means NULL in the eligibility LEFT JOIN at :1552, so the agent was already due, and inserting NULL preserves that rather than granting a first-ever owner test the power to defer. Good catch.

Agreed on the floor being out of scope, and your read of why is right — deferComplianceCheckAfterInconclusiveTarget records no run, so a naive floor cannot tell a deferral from an in-flight lock and would re-select every hour. Happy to file that separately if there's appetite; it wants the queue-state visibility in AAO #616 first, otherwise there is nothing to verify a floor against from outside.

[bokelley]

The fail-closed reasoning is correct, and the two-place catch is important — thank you for both.

Keeping !=: TriggeredBy (line 22) is 'heartbeat' | 'manual' | 'webhook' | 'owner_test' — a TypeScript union that constrains callers at compile time, so a new trigger type can't reach the SQL path without updating the union first. That reduces the fail-open blast radius to callers that bypass the TypeScript layer (raw SQL, migration logic, a union expansion with no predicate update). @therevoltingx — please add a short inline comment on the != 'owner_test' WHERE predicate noting that the TypeScript union is the compile-time guard; a future TriggeredBy expansion opts into schedule advancement unless this predicate is updated alongside the union.

On manual: the PR description documents it as intentional — admin support runs are deliberate registry checks and should advance the cadence. Switching manual to a schedule-no-op is a separate behavioral decision beyond the stated fix. The admin-run-consumes-requeue case is worth a standalone issue if the owner-displacement framing should extend there; it would want the queue-state visibility in #616 first in any case.

Two-place consistency is preserved as written: the CASE ($7 = 'owner_test' THEN NULL) and the WHERE ($7 != 'owner_test') are both keyed on the same literal, so first-run and steady-state agree.

---

Generated by Claude Code

[EvgenyAndroid]

You're right and my premise was wrong. TriggeredBy is a union at compliance-db.ts:22, and RecordComplianceRunInput.triggered_by at :301 is typed with it, not string.

The two lines I cited were read shapes, not the write path: :1294 is getRecentOperationalRuns' return type and registry.ts:1511 is the API response schema. Neither constrains what reaches the CTE. I should have checked the input interface, which is thirty lines from the SQL I was quoting.

webhook is likewise declared, at that same line 22 — so "not a triggered_by value anywhere" was wrong. I checked registry-api.ts and code search and did not grep the type file, which is the exact failure mode I flagged for you two comments earlier. Apologies for the noise.

With a compile-time union the fail-open argument is much weaker, and keeping != is reasonable on that basis. The residual is the one you already named and commented in the code, which is the right place for it.

One factual note on scope, not an argument to change anything: webhook is declared but no caller passes it — zero occurrences as a value in registry-api.ts and compliance-heartbeat.ts, and in compliance-db.ts it appears only in the union and one doc comment. So today the two predicates diverge on manual alone.

Agreed that manual is a separate behavioral decision and out of scope here. I'll leave it unless the owner-displacement framing turns out to extend to admin runs, and I won't open an issue for it on my own read.

On the two-place point, I think we're agreeing: the CASE and the WHERE are consistent as written, and my note was only that they must move together if anyone flips one later. Nothing to change in the PR.

I don't see 0928a9c on the PR yet — still showing a9d9b34a4 as the only commit. Probably just not pushed.

[therevoltingx]

@bokelley — inline comment added in c9a9110 on the WHERE $7 != 'owner_test' predicate, noting the TriggeredBy union as the compile-time guard, that a new member opts into schedule advancement unless the predicate is updated alongside it, and that the INSERT-branch CASE is keyed on the same literal and must move with it. The push dismissed the earlier approvals, so this will need a re-approve.

@EvgenyAndroid — 0928a9c never existed; nothing had been pushed when you looked. c9a9110 is the one. Integration file still 15/15 and typecheck clean on the new head.

[aao-secretariat]

Ladon verdict: Approve

Approve — subsequent pass on PR #7691, clean diff with no findings.

What I checked:

• Delta since the prior clean approve is comment-only: a five-line explanatory comment in server/src/db/compliance-db.ts documenting the $7 = 'owner_test' scheduling predicate in the schedule_next CTE. No executable code changed.
• Reviewer surfaced no critical/high/medium findings.
• No protocol surface touched (static/schemas/source/**, docs/reference/**, dist/** untouched) — no changeset, oneOf, or schema↔docs coherence concerns apply.
• high_risk: false; gated_paths: false (row 2 does not fire even though review_decision is REVIEW_REQUIRED — the gate is path-based and no gated path was touched).
• Author is not on a no-auto-approve team.
• Prior decision was approve, so no sticky-escalation logic applies.

No rows 1–8 fire; falls through to row 9 → approve.

[EvgenyAndroid]

@andybevan-scope3 — this needs a re-approve when you have a moment. Your approval from 19:05 on the 25th was auto-dismissed by the c9a9110 push, which only added the SQL comment @bokelley asked for. No behavioural change since you looked.

State as of now: all four checks green (GitGuardian, IPR signature, check, code_review), mergeable: true, mergeable_state: blocked. aao-secretariat[bot] re-approved at 20:19 but that does not satisfy the human gate, so the PR has sat untouched since 20:20 on the 25th.

Worth flagging that it gates other work: #7693 wants to write last_heartbeat_success_at inside the same schedule_next CTE, and this PR is what threads triggered_by in as $7. Landing this first means that change needs no new call site.

[therevoltingx]

Updated the branch with main. The Build Check failure was the published compliance check packing @adcp/sdk@beta, which no longer exists on npm. #7861 dropped beta from npm_tags after this branch was cut.

After the merge, three of the new tests failed because #7844 now only selects agents with a registry source, and the fixtures were metadata-only. I registered each fixture agent on the suite's member profile, as beforeAll already does. The fix itself is unchanged, and with it reverted the four owner-test cases still fail.

One interaction to flag: #7844 also added compliance_inconclusive_streak = 0 to the same DO UPDATE, so an owner test no longer resets the inconclusive backoff either. A requeue still resets it. Happy to change that if you'd rather a conclusive owner test clear the streak.

[aao-secretariat]

Re-approval: no reviewable delta since the prior approval; re-submitting after stale-review dismissal.

[bokelley]

Leave it as-is — compliance_inconclusive_streak = 0 gated by the same $7 != 'owner_test' predicate is the right call.

The streak is backoff state for the heartbeat's internal cadence. An owner test's conclusive verdict already reaches the owner through the public card and last_checked_at; whether the independent heartbeat should then close its backoff window faster is a question about the heartbeat's state machine, not the run's verdict. A requeue is the right owner mechanism there — it explicitly requests a fresh independent check, and that check clears the streak on completion.

The alternative (let a conclusive owner test zero out the streak but not advance the schedule) requires splitting the WHERE into per-column conditions, which adds surface to an already nuanced predicate. The gain — one fewer heartbeat cycle in backoff after a conclusive owner test — doesn't justify the complexity here.

If the owner-displacement framing later extends to "owner should be able to unstick the backoff without a full requeue," that scopes cleanly as a follow-up: a dedicated clear_backoff action, or a WHERE condition keyed on verdict rather than trigger. Either way it doesn't belong in this fix.

---

Generated by Claude Code