Skip to content

fix(compliance): require explicit heartbeat completion before publication - #7506

Closed
bokelley wants to merge 1 commit into
mainfrom
fix/7404-heartbeat-completeness
Closed

bokelley wants to merge 1 commit into
mainfrom
fix/7404-heartbeat-completeness

Conversation

@bokelley

@bokelley bokelley commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

A hosted heartbeat must finish its selected suite before it can replace a public grade. This closes the remaining fail-open completeness handling after #7453: an absent SDK field was defaulted to complete, and unfamiliar values could not be persisted in the existing closed DB vocabulary.

Heartbeats now require explicit completeness === 'complete'. Other values retain audit evidence under the existing timed_out/not_completed states, with the redacted reported value in provenance and dry_run=false, is_authoritative=false. Their persisted rows continue advancing the SDK rotation offset. Public status, complete storyboard rows/counts, current-run association, badges, and notifications remain unchanged. A first incomplete run leaves the agent ungraded, with no materialized verdict or public history entry.

Heartbeat persistence uses the connection that owns its execution fence, so losing that session aborts the publication transaction. Heartbeat storyboard-write failures also roll back the status/run instead of committing a new headline with stale storyboard materialization. If publication fails, the returned SDK evidence is retained as non-authoritative instead of being mislabeled as an unreachable agent. A scheduling failure after persistence does not duplicate the run. Replayed persistence does not repeat heartbeat notification or badge decisions. Owner/manual completeness compatibility and targeted-run semantics are unchanged.

Refs #7404 (Bug 3 / ask 4 only). No migration, protocol changeset, release artifact change, deployment, production-agent probe, or production DB write. This PR does not close all of #7404.

#7404 acceptance Regression evidence
50/75 original truncation and 48/75 recurrence preserve the complete grade/denominator Real heartbeat → adapter → PostgreSQL tests compare raw materialized rows plus public current/bulk/storyboard/history/observation reads; no badge/notification calls.
Absent, malformed, budget_exhausted, partial, and future completeness fail closed Adapter and DB-boundary matrices; the redacted original value is retained in provenance.
First incomplete run has no false verdict; repeated runs rotate No status/storyboard/public-history/badge rows; durable offsets advance, then an explicit complete result becomes public.
Exact complete replacement retains normal side effects The real transaction replaces prior rows and calls transition/badge/verification notification seams once.
Concurrent owner refresh / fence loss Independent PostgreSQL sessions and SQL barriers prove a delayed incomplete commit cannot overwrite newer owner evidence; fence loss rolls back publication.
Evidence/materialization faults and replay Transaction fault injection, immutable audit replay, complete replay, wrong-lease rejection, and zero repeated heartbeat side effects.

Exact rebase provenance

The sole reviewed commit was transplanted from prior base 2da35544ffb7257861324786a33cae9db0457bac / prior head ee428b15aef967c0ce2dd3ab3998a5c1a62b27b4 onto the requested current main. The publication used an explicit force-with-lease expecting that exact prior head. Main's intervening #7533 delta changes exactly three CSRF/MCP paths and has zero path overlap with this PR's ten-path delta. Base deploy run 34925749626 completed successfully before publication.

Item Exact value
Base / parent cdcc8f996e8acd2417d5f7a547711cd21929289b
Head 92cee2031c2aea09f89cfdf6ec581f6711a9985b
Result tree 97051f09aaba397d8400e45b16870ce148cbc630
Stable patch-id (old and new) 4ed1faa8d1d7457a368ba05e47c38018b7a564cd
Binary/full-index/no-renames diff SHA-256 (old and new) c558f4f1d6d15be343b3c7f5b979012c0a95b5b17d82fd54d98ebe55fa26741c
Scope 1 commit; 10 paths; +504/-21

git range-diff maps the commits with =. Old/new name-status and the complete serialized diff are identical. All 10 result path modes and blob IDs are identical. git diff --check is clean, and the rebased commit has exactly the requested parent.

Exact-head local qualification

Run against 92cee2031c2aea09f89cfdf6ec581f6711a9985b:

  • Focused completeness/publication plus fix(auth): allow trailing slashes on MCP CSRF exemptions #7533 CSRF/MCP qualification: 6 files / 183 tests passed. This includes the prior 4 files / 108 tests, 28/28 real-PostgreSQL integration cases, and all 75 tests in the two fix(auth): allow trailing slashes on MCP CSRF exemptions #7533-changed suites covering its three trailing-slash allow, near-miss rejection, and bearer-routing behaviors.
  • Root unit suite: 71 files / 1,063 tests passed.
  • Full server suite: 617 files passed; 9,491 tests passed; 31 skipped (9,522 total).
  • Dynamic-import guards: 7 passed; format-identity boundary: 1 passed; call-API state-change guards: 10 passed.
  • Image-quality guard, version synchronization, changeset protocol-scope check, TypeScript check, and fixed trace-rollout TypeScript check all passed.
  • Independent exact-delta review is clean: no Must Fix, Should Fix, Consider, correctness, security, or publication-boundary findings. It independently reproduced the range/patch/blob/diff identity proof, confirmed zero fix(auth): allow trailing slashes on MCP CSRF exemptions #7533 interaction, and passed a separate 4 files / 135 tests targeted subset.

Fresh exact-head CI

TERMINAL GREEN for exact head 92cee2031c2aea09f89cfdf6ec581f6711a9985b. All 32 current check-run records have that exact head_sha: 31 succeeded and 1 expected Draft code_review skip, with zero pending, failing, or cancelled records. The separate IPR Policy / Signature commit status succeeded, and all required checks passed. The fresh build/test run passed all four canonical shards, all four server-unit shards, all four server-integration shards, build/static checks, and the unit/integration aggregates. CodeQL and dependency/security scans passed, and no CodeQL review comment was present at terminal inspection.

This CI result does not change readiness: the PR remains Draft and no merge claim is made.

Readiness event provenance

At 2026-09-15T02:37:41Z, the mutation owner incorrectly changed the PR to ready-for-review after interpreting “Draft off” as disabling Draft mode. The coordinator restored Draft at 2026-09-15T02:38:03Z, a 22-second ready-for-review window. The PR remains Draft; it will not be toggled again in this task. Auto-merge stayed off, and no merge occurred.

GitHub's displayed APPROVED review decision is based only on bot reviews and does not count as human maintainer approval. After terminal exact-head CI, the maintainers team review request was renewed at 2026-09-15T04:12:38Z; human approval remains pending. No merge, deployment, release, R2/web/worker operation, live-agent verification, successor-PR mutation, or #7404 closure is performed here.

@bokelley
bokelley requested a review from a team September 14, 2026 12:59
@bokelley
bokelley marked this pull request as ready for review September 14, 2026 13:02

Copy link
Copy Markdown
Contributor Author

@adcontextprotocol/maintainers — this PR is now ready for human review at 37624824fe713b8666de7fe4cf3556c3b8f3d9fc, based on current main 71f9cd5414454e94ccfef87ce25777ead6fad228 (rechecked before leaving Draft). The independent second-session exact-head code/security review is clean. Exact-head CI finished with 32 passed / 2 skipped, including 9,386 server unit tests and 2,133 integration tests passed. Please review the Bug 3 publication, evidence and execution-fence changes. The inherited RC.3 controller-requirement discrepancy is documented separately; release artifacts are unchanged by this PR. Auto-merge remains off.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean, well-tested fail-closed fix for hosted-heartbeat completeness publication (#7404 Bug 3).

What I checked:

  • Heartbeats now require explicit completeness === 'complete' to publish; unknown/absent SDK values normalized to not_completed and retained as audit-only.
  • Triple-enforced closed completeness vocabulary (adapter, publication predicate, DB write boundary).
  • Provenance redaction, fence-owned publication transaction with commit/rollback atomicity, no-duplicate audit evidence guards.
  • Correct absence of a changeset — this is Addie/server compliance code (non-protocol surface), so no .changeset/*.md is required.

No blocking findings. No Critical/High/Medium findings reported.

Notes on gates:

  • high_risk is true only for two (modified) files under server/addie/**, but the reviewer found no medium-or-higher concerns, so the modifications are presumed safe (row 5 does not fire — no medium finding).
  • gated_paths is false; no no-auto-approve team match; first review.

Rows 1–8 do not fire → row 9 approve.

@bokelley
bokelley force-pushed the fix/7404-heartbeat-completeness branch from 3762482 to 19e6af0 Compare September 14, 2026 13:25

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean pass on a server-only change to Addie's compliance heartbeat publication (#7404 Bug 3).

No blocking findings. The reviewer found no critical/high/medium concerns. Changes cover fail-closed completeness closure (heartbeatRunCompleteness), fence-owned transactional atomicity in recordComplianceRun, commit-or-rollback heartbeat storyboard writes, and a post-grade failure path that retains audit-only evidence without duplicating committed runs.

No protocol surface touched (no static/schemas/source, docs/reference, dist, or task/compliance assets), so no changeset is owed and the changeset/oneOf/immutability/patch-eligibility gates do not fire. Owner/manual callers preserve verbatim completeness via explicit triggered_by.

Decision-table walkthrough:

  • No critical/high findings (row 1 n/a).
  • gated_paths is false; review_decision is APPROVED anyway (row 2 n/a).
  • high_risk is true but both high-risk reasons are (modified) with no medium findings — no (deleted), and rows 4/5 require a medium finding, of which there are none.
  • Prior decision was approve, so sticky escalation (row 6) does not apply.
  • No no-auto-approve team match (row 7 n/a).
  • Zero medium findings, well under the three-medium threshold (row 8 n/a).
  • Falls through to row 9 → approve.

Copy link
Copy Markdown
Contributor Author

@adcontextprotocol/maintainers — fresh human review requested after rebasing onto current main and completing exact-head review/CI.

  • Head: 19e6af0a278a383ddc5066c38c3d298f6f5baf10
  • Base/parent: af1ea2666d77a56e287567567daa72a301562e52
  • Tree: cc94135f85d5f9144feaaee7e31ff3a80505a0c1
  • Stable patch-id: 4ed1faa8d1d7457a368ba05e47c38018b7a564cd, identical to the prior reviewed patch; 10 files, +504/-21.

#7504 has no patch-file overlap; its source webhook applicability regression and the focused/root/six-file PostgreSQL gates passed. Fresh independent Codex code/security session 93a3894e-1c52-4ec6-b475-413d3d9a6e88 found no blocker. Exact-head CI is terminal green: 33 checks passed; unit 9,386 passed / 31 skipped, integration 2,133 passed / 8 skipped. GitHub's CI merge tree matches the reviewed tree. Fresh Ladon approval is recorded separately; human review is still requested.

The immutable RC.3 controller/webhook source gaps remain outside this Bug 3 patch. No live production-agent/card validation was performed; SDK timing is mocked. Auto-merge remains off. Please review; do not merge through this agent session.

@bokelley
bokelley requested review from a team and removed request for a team September 14, 2026 13:36
@bokelley
bokelley force-pushed the fix/7404-heartbeat-completeness branch from 19e6af0 to 573db63 Compare September 14, 2026 14:34

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean subsequent review of PR #7506.

No blocking findings. This head's patch-id is identical to the previously-approved head, and the change is fail-closed and well-tested: heartbeat publication now requires explicit completeness === 'complete', normalizing absent/unknown SDK values to non-authoritative audit evidence, with fence-owned persistence that aborts the transaction on lease loss. This is a server-only compliance-worker change with no protocol-surface, dist, or changeset-scope impact.

Gate checks:

  • No no-auto-approve team match.
  • gated_paths: false; review_decision: APPROVED.
  • high_risk: true, but only via (modified) entries under server/addie/** with zero medium-or-higher findings — presumed safe per the high-risk-flag reading. No deletions, no schema/dist/data-loss concerns.
  • Prior decision was approve; row 6 sticky escalation does not apply.

No critical/high/medium findings → rows 1–8 do not fire → row 9 approve.

@bokelley
bokelley requested review from a team and removed request for a team September 14, 2026 14:51

Copy link
Copy Markdown
Contributor Author

@adcontextprotocol/maintainers — fresh human review requested after the #7494 rebase, independent exact-delta review and green exact-head CI.

  • Head: 573db6359b85307712ae2b74aa3c1840c6c4db1b
  • Base/parent: 98fa207cb81c14b745f66a90d0b82cb6fad82f4b
  • Tree: 5bf8490c9d72f3bacaab38db04a0fca23b0ce060
  • Stable patch-id: 4ed1faa8d1d7457a368ba05e47c38018b7a564cd. All ten previously reviewed production/test blobs are unchanged; +504/-21, range-diff =.

Independent Codex session 93a3894e-1c52-4ec6-b475-413d3d9a6e88 found no blocker or security issue at this exact head. CI is now 33 checks passed: 9,403 unit tests / 31 skipped, 2,133 integration tests / 8 skipped. Local grader/publication 362, PostgreSQL 80, training 1,313 and root 1,063 tests passed; exact-base control passed 28 tests.

Initial C2PA installation and Bun HTTP 504 failures remain documented in the body. Diagnosed, scoped retries passed without any source/test edit. The exact-base static job passed; its separate private-ledger assertion failures are distinguished from this candidate. Fresh Ladon approval is recorded separately from this human request.

Auto-merge remains off. No merge/deployment was performed. This addresses #7404 Bug 3 only; it does not close the entire issue or verify live public cards.

@bokelley
bokelley requested review from a team and removed request for a team September 14, 2026 15:04
@bokelley
bokelley force-pushed the fix/7404-heartbeat-completeness branch from 573db63 to f0a6290 Compare September 14, 2026 16:34

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean pass on PR #7506 (post-rebase, diff unchanged from prior approve).

No blocking findings. The heartbeat-completeness fix is fail-closed at both the adapter and DB boundary, commits grade + storyboard denominator atomically on the fence-owning connection, retains non-authoritative evidence on post-run failures, and is covered by real-DB regression tests.

Checks:

  • App-only change under server/; no static/schemas/source/ touched, so no schema↔docs coherence, oneOf discriminator, or changeset-scope concerns.
  • No released-artifact (dist/**) modifications.
  • high_risk is true only via two (modified) files under server/src/addie/**, but reviewer surfaced no medium-or-higher findings on them, so the modifications are presumed safe (no escalation on the flag alone).
  • gated_paths: false; review_decision: APPROVED.
  • No no-auto-approve team match.

Decision table: rows 1–8 do not fire (no critical/high/medium findings, no deletions, no gated-paths gap, no team gate). Falls through to row 9 → approve.

@bokelley
bokelley requested review from a team and removed request for a team September 14, 2026 17:24

Copy link
Copy Markdown
Contributor Author

@adcontextprotocol/maintainers — fresh review requested after the main merge wave settled.

Head f0a62909a64213657df2afe754492f8357ee2e71, parent/base 36a86d6c4ae5c42e5e2e783d95bdb97b5cb6f614, tree 298e361c8a2371a293614b32e49d1ab80f2a46af. All ten reviewed production/test blobs and stable patch-id are unchanged; range-diff is =. Independent exact-delta review in session 93a3894e-1c52-4ec6-b475-413d3d9a6e88 is clean.

Exact CI is terminal green: 33 passed, zero pending/failing. Build/test and Ladon review passed on attempt 1; the cancelled pre-execution IPR check passed on its diagnosed targeted rerun. Full suites: 9,432 unit and 2,133 integration tests passed; 31 unit and 8 integration tests skipped. The body preserves failure history and coverage limits, including no live affected-agent validation and the unfaulted diagnostics savepoint.

Live main/head were rechecked before this formal renewal. Human approval remains pending; PR is non-Draft, auto-merge off. No merge/deploy or claim that all of #7404 is closed.

Copy link
Copy Markdown
Contributor Author

Coordinator safety hold: temporarily Draft with auto-merge off. The repository currently permits author/self merge without an exact-head human approval even when Ladon requests human review. This PR remains technically green and unchanged, but must receive an exact-head human maintainer approval before Ready/merge/deploy. #7507 remains separately gated on this deployment and corrected #7510.

aao-secretariat[bot]
aao-secretariat Bot previously approved these changes Sep 15, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — clean subsequent review on PR #7506, no blocking findings.

What I checked:

  • No author no-auto-approve team match; gated_paths is false; review_decision is APPROVED — no hard gates apply.
  • high_risk is true only for two (modified) files under server/src/addie/** (compliance-heartbeat.ts, compliance-testing.ts). Per the change-kind rules, modifications with no medium-or-higher findings are presumed safe; row 5 does not fire because there are zero medium findings.
  • No (deleted) high-risk entries (row 3 N/A).
  • Reviewer reports zero Critical/High/Medium findings on the full-PR delta: heartbeat publication now requires completeness === 'complete', double-guarded at adapter and DB boundary, fail-closed on unknown SDK values. Fence-owned persistence, atomic storyboard/status rollback, replay idempotency, and non-authoritative labeling of graded-then-failed runs verified.
  • Operational/server-only change — no static/schemas/source/**, docs/reference/**, or dist/** surface touched, so no changeset required and no schema↔docs coherence concern.
  • Prior decision was also approve with no findings; this is a net-clean continuation.

No decision-table row 1–8 fires → row 9 (approve).

@bokelley
bokelley requested review from a team and removed request for a team September 15, 2026 02:45
@bokelley
bokelley force-pushed the fix/7404-heartbeat-completeness branch from ee428b1 to 92cee20 Compare September 15, 2026 04:03
@bokelley
bokelley requested review from a team and removed request for a team September 15, 2026 04:12
bokelley added a commit that referenced this pull request Sep 15, 2026
Require validate_input for CTV experience and premium display validation while
retaining controller workflow prerequisites. Verify source and generated bundle
selection, unavailable-fixture neutrality and genuine validator failures.

Reconcile only the 32 controller seed passes removed from brand and governance
in both synchronized matrix floors. Prove the tenants lack the validator through
live tool discovery and exact old-gate adapter replay. Preserve all other floors.

Related issue: #7404

Required order: #7506 web+worker deploy, #7510, refreshed #7507 deploy, then
public-card verification. This patch alone does not complete the tracking issue.

Copy link
Copy Markdown
Contributor Author

Rebased onto current main at e3693a44b; new exact head is 5ad01bf294569ecc2344ee6c85e176652ca2915c. Conflict resolution preserves the newer grading-profile assessment/advisory-lock and operational-diagnostics work while retaining the heartbeat execution-fence transaction and completeness checks.

Local validation:

  • 83 focused unit tests pass (compliance-heartbeat, compliance-publication, compliance-db-last-write-wins)
  • full TypeScript typecheck passes
  • integration suite was attempted but this workspace has no PostgreSQL listener (ECONNREFUSED 127.0.0.1:58433), so CI is authoritative
  • pre-push storyboard matrix was bypassed only after it failed every tenant for the same base-environment mismatch: local training agent advertises through 3.2-rc.3 while current main builds 3.2.0-rc.4; this is unrelated to this PR

Keeping Draft. Fresh exact-head human maintainer review is required before Ready/merge/deploy; the prior approvals predate this head.

Copy link
Copy Markdown
Contributor Author

Queue cleanup: superseded by the later hosted-grading liveness stack, especially merged #7622 (7de1668e). Current main now fences concurrent full-suite execution, classifies incomplete/not-completed heartbeat runs as audit-only, preserves the last authoritative public grade, defers inconclusive target selection, and exposes bounded operational diagnostics. Rebasing this older duplicate would conflict across the same heartbeat/scheduler/publication paths without adding the missing behavior. Closing in favor of the landed implementation.

@bokelley bokelley closed this Sep 23, 2026
bokelley added a commit that referenced this pull request Oct 6, 2026
…#7541)

Require validate_input for CTV experience and premium display validation while
retaining controller workflow prerequisites. Verify source and generated bundle
selection, unavailable-fixture neutrality and genuine validator failures.

Reconcile only the 32 controller seed passes removed from brand and governance
in both synchronized matrix floors. Prove the tenants lack the validator through
live tool discovery and exact old-gate adapter replay. Preserve all other floors.

Related issue: #7404

Required order: #7506 web+worker deploy, #7510, refreshed #7507 deploy, then
public-card verification. This patch alone does not complete the tracking issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant