Repository navigation
Conversation
|
@adcontextprotocol/maintainers — this PR is now ready for human review at |
There was a problem hiding this comment.
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 tonot_completedand 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/*.mdis required.
No blocking findings. No Critical/High/Medium findings reported.
Notes on gates:
high_riskis true only for two(modified)files underserver/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_pathsis false; no no-auto-approve team match; first review.
Rows 1–8 do not fire → row 9 approve.
3762482 to
19e6af0
Compare
There was a problem hiding this comment.
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.
|
@adcontextprotocol/maintainers — fresh human review requested after rebasing onto current main and completing exact-head review/CI.
#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 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. |
19e6af0 to
573db63
Compare
There was a problem hiding this comment.
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 underserver/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.
|
@adcontextprotocol/maintainers — fresh human review requested after the #7494 rebase, independent exact-delta review and green exact-head CI.
Independent Codex session 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. |
573db63 to
f0a6290
Compare
There was a problem hiding this comment.
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.
|
@adcontextprotocol/maintainers — fresh review requested after the main merge wave settled. Head 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. |
|
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. |
f0a6290 to
ee428b1
Compare
There was a problem hiding this comment.
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 underserver/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/**, ordist/**surface touched, so no changeset required and no schema↔docs coherence concern. - Prior decision was also
approvewith no findings; this is a net-clean continuation.
No decision-table row 1–8 fires → row 9 (approve).
ee428b1 to
92cee20
Compare
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.
92cee20 to
5ad01bf
Compare
|
Rebased onto current Local validation:
Keeping Draft. Fresh exact-head human maintainer review is required before Ready/merge/deploy; the prior approvals predate this head. |
|
Queue cleanup: superseded by the later hosted-grading liveness stack, especially merged #7622 ( |
…#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.
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 existingtimed_out/not_completedstates, with the redacted reported value in provenance anddry_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.
budget_exhausted,partial, and future completeness fail closedExact rebase provenance
The sole reviewed commit was transplanted from prior base
2da35544ffb7257861324786a33cae9db0457bac/ prior headee428b15aef967c0ce2dd3ab3998a5c1a62b27b4onto 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.cdcc8f996e8acd2417d5f7a547711cd21929289b92cee2031c2aea09f89cfdf6ec581f6711a9985b97051f09aaba397d8400e45b16870ce148cbc6304ed1faa8d1d7457a368ba05e47c38018b7a564cdc558f4f1d6d15be343b3c7f5b979012c0a95b5b17d82fd54d98ebe55fa26741cgit range-diffmaps 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 --checkis clean, and the rebased commit has exactly the requested parent.Exact-head local qualification
Run against
92cee2031c2aea09f89cfdf6ec581f6711a9985b:Fresh exact-head CI
TERMINAL GREEN for exact head
92cee2031c2aea09f89cfdf6ec581f6711a9985b. All 32 current check-run records have that exacthead_sha: 31 succeeded and 1 expected Draftcode_reviewskip, with zero pending, failing, or cancelled records. The separateIPR Policy / Signaturecommit 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 at2026-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
APPROVEDreview 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 at2026-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.