Repository navigation
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — fail-closed supply-path verifier rewrite, clean review.
Reviewer confirmed the load-bearing fix (host grants now require an affirmative collections !== undefined selector; verified_owner_sold requires all three legs with full property coverage and no unsupported constraints), bounded fail-closed IAB fetching (5s / 256KB / 0-redirect / text-plain + rate limiter), DoS cost bounds, single-level bulk recursion, and schema↔docs↔OpenAPI coherence for semantics_version / resolved_collection_id. Changeset present, minor, protocol-scoped; additive wire fields; 64-vector golden corpus and unit tests included.
No Critical/High/Medium findings.
Gate checks:
- Author team gates: no no-auto-approve match.
- gated_paths: false → row 2 does not fire.
- high_risk: true, but only
server/src/routes/registry-api.ts (modified)with no medium-or-higher finding on it → rows 3 and 5 do not fire. - No data-loss/schema/infra medium findings (row 4 n/a).
- No prior decision (row 6 n/a).
- Zero findings → rows 1, 8 n/a.
Falls through to row 9 → approve.
87b09d1 to
f701ede
Compare
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated-path approval gate not satisfied.
This PR modifies protocol schema source (static/schemas/source/adagents.json and static/schemas/source/core/collection.json), which places it under the deterministic hard approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so per decision rule row 2 the outcome must be escalate rather than approve — despite the reviewer surfacing no critical/high/medium findings.
Escalation reasons
static/schemas/source/adagents.json(modified) — under hard approval gate; requires human/CODEOWNERS approval (review_decisionisREVIEW_REQUIRED).static/schemas/source/core/collection.json(modified) — under hard approval gate; requires human/CODEOWNERS approval.
Note: the reviewer confirmed fail-closed verifier semantics, same-origin/SSRF redirect policy, revocation persistence, provenance migration 591, and schema↔docs/changeset coherence with no blocking or Medium findings. The two non-blocking follow-ups (a benign publisher-migration pin surfacing as a 500, and the full precommit suite not yet confirmed green on final source) are informational. Once a human/CODEOWNERS approval lands (review_decision: APPROVED), this clean diff would fall through to approve.
Why human review
- static/schemas/source/adagents.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
- static/schemas/source/core/collection.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches
static/schemas/source/**; static/schemas/source/core/collection.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS approval.
This PR modifies published protocol source under static/schemas/source/** (adagents.json and core/collection.json), which is under the repo's hard, deterministic approval gate. gated_paths is true and review_decision is REVIEW_REQUIRED (not APPROVED), so decision-table row 2 forces escalation — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
The reviewer delta pass surfaced no Critical/High/Medium findings (fail-closed hardening of the supply-path authority surface with matching normative docs and new tests), so there are no blocking code findings. This escalation is purely the schema-source approval gate.
Escalation reasons
static/schemas/source/adagents.json(modified) — under the hard approval gate; review_decision is REVIEW_REQUIRED, human/CODEOWNERS approval required.static/schemas/source/core/collection.json(modified) — under the hard approval gate; review_decision is REVIEW_REQUIRED, human/CODEOWNERS approval required.
Why human review
- static/schemas/source/adagents.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
- static/schemas/source/core/collection.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches
static/schemas/source/**; static/schemas/source/core/collection.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate not yet satisfied.
This PR modifies protocol schema source (static/schemas/source/adagents.json, static/schemas/source/core/collection.json), which is under the repo's hard, non-overridable approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so row 2 of the decision table fires: Ladon cannot auto-approve regardless of how clean the diff is until a human/CODEOWNERS approval is recorded.
Blocking findings: none. The reviewer reported a clean pass on the delta — the change swaps .hash for parsed.href.includes('#') at three supply-path trust-boundary sites (adagents-manager.ts:607, supply-path-authority-state.ts:458, supply-path-snapshot.ts:661) to close the bare-# empty-fragment bypass, each with a matching test. Fail-closed, strict superset of the prior check, no false positives on %23-encoded paths. No critical/high/medium findings.
Medium findings: none.
Escalation reasons:
static/schemas/source/adagents.json(modified) is under the hard approval gate; review_decision isREVIEW_REQUIRED— human/CODEOWNERS approval required.static/schemas/source/core/collection.json(modified) is under the hard approval gate; review_decision isREVIEW_REQUIRED— human/CODEOWNERS approval required.
This matches the prior decision (escalate) — the gate persists purely because the required human/CODEOWNERS approval has not yet been recorded. Once review_decision becomes APPROVED, this clean diff can fall through to approve.
Why human review
- static/schemas/source/adagents.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
- static/schemas/source/core/collection.json (modified) is under the hard approval gate and review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/adagents.json (modified) matches
static/schemas/source/**; static/schemas/source/core/collection.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
|
Coordinator migration-collision hold: auto-merge disabled and PR moved to Draft. This branch publishes migration 591_supply_path_manifest_provenance.sql, but #7457 already reserved/published migration 591 as 591_compliance_refresh_authenticated_credential.sql. Merging either order would create a duplicate migration number/startup-deploy collision. Please renumber this PR to 597 or higher only after a fresh main/open-PR allocation audit; preserve 591-596 ownership/order, rerun migration-ledger/replay CI and obtain exact-head independent + human review before Ready. |
|
Renumbered: Remaining before Ready:
PR stays Draft until those gates clear. Generated by Claude Code |
|
Coordinator correction to the renumber note: 597 is currently uncontested, but the statement that no open PR claims 592–596 is false. #7463 owns 592, #7464 owns 593 (#7460 inherits the same 593), unpublished Slack lifecycle reserves 594, #7501 owns 595, and unpublished grant lifecycle reserves 596. Preserve all of those allocations. The transient 7956c26 head also briefly contained both 591 and 597; latest 70345d0 appears to remove 591, but remains under independent audit and is still based behind current main 1467e46. Keep Draft/auto-off; do not claim exact-head readiness until the latest tree, rebase, CI, body and full allocation references are corrected. |
Migration 591 was already reserved by #7457 (591_compliance_refresh_authenticated_credential.sql). Renumbering to 597, the next uncontested slot after a main/open-PR allocation audit (main tops at 590; no open PRs claim 592–596). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017jbrQJh44D9RmYbz2yM6pY
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017jbrQJh44D9RmYbz2yM6pY
70345d0 to
f4049f8
Compare
|
Closing as superseded by merged #7544, which deliberately combines the work from #7513 and #7514. This Draft is now stale, conflicting, and duplicates code already on |
The supply-path verifier could report a verified path with absent or malformed collection constraints, dangling carriage references, or grants narrowed by unsupported qualifiers. Domain-level queries could combine unrelated collection legs.
This change fails closed across the five-leg verdict, publishes canonical shared golden vectors with
semantics_version: "1", and preserves authorization bulk-grant semantics. It verifies complete property binding and independently evaluates collection-specific IAB evidence with bounded fetching and exact-origin redirects.The registry persists authority-scoped revocation holds and authoritative-location pins through the existing SDK PostgreSQL state store. Migration 597 binds successful-fetch provenance atomically to each cached manifest, keeping failed-attempt URLs and truncated display URLs out of authorization decisions. Existing cache rows require a successful refresh before establishing authority. Cross-origin shared documents require explicit publisher attribution. Sources expose observation times and full resolved URLs. Malformed provenance, mixed pointers, and chained targets fail closed without discarding held denials. First pins require a successful, valid manifest write.
Dependency: merge this upstream PR before adcp-client#2908, completing adcp-client#2783. The client consumes the canonical vector bytes, rejects older semantics, and must pin the actual merged upstream SHA before its own merge. The prior SDK dependency adcp-client#2911 has merged as
8ba12c2ace85a88533ce1d56efd35badea51a97c; its repair does not replace this head's fresh Generate TypeScript SDK check.Migration allocation
The final tree contains only
597_supply_path_manifest_provenance.sql, with unchanged SQL blobf6629447ccfecb28834380e9efe4863099454555. The old supply-path 591 filename is absent. The obsolete intermediate two-file commit and its canceled CI are not qualification evidence.588_agent_compliance_runner_version.sql589_compliance_run_publication.sql590_compliance_run_provenance.sql591_compliance_refresh_authenticated_credential.sql592_email_mutations.sql593_admin_credential_bind_operations.sql594_slack_identity_binding_operations.sql595_normalized_email_invariant.sql596_organization_credential_grant_lifecycle.sql597_supply_path_manifest_provenance.sqlAt 2026-09-14 18:38:35 UTC, main and the then-open 50 PR migration directories were audited, together with the coordinator's unpublished 594/596 reservations. That snapshot found no other published PR claiming 597. The subsequent independent audit counted 51 open PRs; the newcomer had no migration. Neither snapshot inventories undisclosed worktrees, and unpublished 594/596 reservations are coordinator-supplied evidence rather than live GitHub refs. The earlier claim that no open PR owned 592–596 was incorrect; the table above is the corrected allocation.
597 has no SQL/application dependency on the #6827 chain. Preserve that chain's internal 591 → 592 → 593 → 594 → 595 → 596 order. Its files are not imported or modified in this PR. See allocation and replay evidence for exact reserved hashes and ownership.
Validation and review
fc41b86e, which is not GitHub-retrievable and is not immutable exact-head CI evidence. All server runtime/test files are identical through this head except one removed EOF blank line. Main's subsequent feat(compliance): frequency-cap proposal-refinement scenario #7522 adds only a compliance scenario and its changeset.f4049f8c/ tree44f9c3427847da8844eb1d91ca6d5214746aa3fb, preserved behavior with evidence-file additions and EOF cleanup, unchanged protected SQL blobs, and applicability of the recorded local migration evidence. This does not replace human/CODEOWNERS approval.Exact-head GitHub CI is terminal: 61 successful checks, 2 failures, 1 intentionally skipped (Ladon for Draft). All storyboards, both generated SDKs, IPR, migration smoke/duplicate checks, and build/static checks passed. TypeScript SDK generation consumed the
f4049f8cprotocol bundle against client mainecf1c745. The new provenance migration regression also executed and passed 2/2 tests in fresh integration shard 2/4. The only substantive failure is private-ledger integration shard 3/4; its aggregate is the second red check. Content/migration audit: PASS. Merge readiness: BLOCKED. The push used an explicit lease against70345d00df6baabf8828709965a599da861fecbe; Draft, auto-merge off, and the bmilekic review request were verified afterward. The local advisory storyboard matrix was skipped; the fresh exact-head GitHub matrix is authoritative. Older head70345d00failed integration shard 2/4 with two 30-second external-agent smoke timeouts atexternal-smoke-check.test.ts:35,45(504 passed / 2 failed). The migration correction does not fix that unchanged live harness; #7511 owns its separate repair and remains held. No code from that PR is copied here, and old CI is not counted as final qualification.Current-head failure classification: job 104103521658 failed
fixed-trace-component-smoke-private-ledger-migration.test.ts:488(637 passed / 1 failed / 7 skipped). PostgreSQL backend 211 logged a lock timeout at 18:43:28.900Z on the complete-plan recoverySELECT ... FOR UPDATE. The test holds the plan row across two fixed 75 ms sleeps against the unchanged production 250 ms lock budget; scheduling overhead can correctly producepersistence_uncertain. The test, implementation, and migrations 582/583 are byte-identical to main. This is the race already repaired by held #7514, not #7511's external smoke harness. No rerun or repair copy was performed. Consume #7514 only after its current-main qualification and human-approved landing, then rerun exact-head qualification. A probabilistic green retry would not incorporate that repair.