Repository navigation
Conversation
bokelley
marked this pull request as ready for review
October 31, 2026 01:10
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — codegen-tooling backport (scripts + tests only).
Checked:
- No findings from the reviewer (critical/high/medium/low all clear).
- Diff touches only
scripts/generate_types.py,scripts/post_generate_fixes.py, and three test files — no publicadcp.*surface, no wire-shape change, generated output claimed byte-identical for the 3.2.7 pin. - New
pin_conflicting_allof_propertieslogic is fail-closed (raises on undecidable oneOf/anyOf-branch disagreement rather than guessing), andstabilize_inlined_core_refsrewrites resolve correctly via existing_normalize_schema_ref_target. find_class_header/require_class_headerandnormalize_emitted_envelope_basescorrectly follow generated class composition; strong new test coverage.fix(codegen)semver signal is correct for a non-breaking tooling change.
Gate checks: no no-auto-approve team match; gated_paths false; high_risk false. No blocking or medium findings → decision table falls through to row 9 (approve). Note: review_decision is REVIEW_REQUIRED, but with gated_paths: false row 2 does not apply.
bokelley
marked this pull request as draft
October 31, 2026 01:25
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Optional maintenance fallback: adcontextprotocol/adcp#8636 selects this SDK's
nextbranch for 3.3 schema CI, where both SDK generation jobs now pass. This stable-line backport is retained as a validated alternative and is not required for that integration path.The spec repository's current default-branch checkout cannot generate the composed 3.3 schema tree because the repairs in #1705 and #1741 landed on
next.Backport both repairs: resolve inlined core references at every caller depth, retain composed envelope ancestry and narrowed fields, and locate generated classes by name. The response-emitter conflict preserves
main's existing explicit-null handling. The SDK stays pinned to 3.2.7 and its generated output remains byte-identical.Validation in Docker:
make validate-generatedconfirms byte-identical output.make regenerate-schemas,make validate-generated, and the same 156 tests pass. This includes export consolidation, envelope ancestry, narrowed fields and version-field retention.maingenerator.The candidate run mirrors spec CI's disposable pin/manifest adjustment and candidate-only sync isolation. It uses the deterministic bundle built at adcontextprotocol/adcp commit
95e1447549daf96ea4f29b9a879f7fe4090a4a59, including #8547, with SHA-256d3a612e34b0e6ddb533f7d5c867ddccca9697f4734f743bfde0f2fba924876c7. This validates the current source bundle; the existing published beta.2 artifact remains unchanged.Refs #1680 and adcontextprotocol/adcp#8547. The implementation is backported from #1705 and #1741.