fix(tests): remove stale strict xfails on get-products schema drift#1336
Merged
Conversation
PR #1334 added strict xfail markers asserting the get-products-request schema drift would fail; PR #1276 then added allowlist coverage (if_catalog_version, if_pricing_version in KNOWN_SCHEMA_LIBRARY_MISMATCHES; adcp_major_version in _VERSION_FIELDS). The combination made the tests xpass, breaking main CI. Two markers removed: - tests/unit/test_pydantic_schema_alignment.py: SCHEMA_TO_MODEL_PARAMS_WITH_GET_PRODUCTS_DRIFT_XFAIL no longer wraps get-products-request with strict xfail. - tests/e2e/test_schema_validation_standalone.py::test_offline_mode: the standalone strict xfail removed. Tracked: the underlying #1308 issue (adcp library doesn't model these fields natively) is still open — the allowlists are the workaround until adcp library upgrades. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mkomorski
approved these changes
May 21, 2026
2 tasks
mkostromin-sigma
added a commit
to mkostromin-sigma/salesagent
that referenced
this pull request
May 22, 2026
… drift The AdCP /schemas/latest/ spec updated again after prebid#1336 removed the strict xfail. The minimal payload {products:[], cache_scope:public} now fails response schema validation because the live spec added new required fields not yet modelled in the adcp library (tracked in prebid#1308). Restore the xfail with strict=False so that: - test FAILS (spec still ahead of library) => recorded as xfail, CI green - test PASSES (spec and library converge) => recorded as xpass, CI green Unlike the strict=True removed in prebid#1336, strict=False tolerates both directions of drift without flipping CI red.
ChrisHuie
pushed a commit
that referenced
this pull request
May 22, 2026
* docs: add SECURITY.md, [project.urls], description Closes PD5, PD23 from issue #1234 rollout. - Add SECURITY.md with private vulnerability reporting policy, disclosure timeline, and CI/hook modification policy - Replace placeholder description in pyproject.toml with meaningful text - Add [project.urls] block (Homepage, Repository, Issues, Documentation, Changelog) * docs: rewrite root CONTRIBUTING.md as thin pointer Per D21: docs/development/contributing.md (594 lines) is canonical. Root CONTRIBUTING.md becomes a ~30-line pointer with quick-start, PR title format summary, and link to SECURITY.md. * chore: add CODEOWNERS Closes PD4. Routes PR review to @ChrisHuie for CI, security, test infrastructure, ratchet baselines, and auth surfaces. * ci: add dependabot.yml (no auto-merge) Closes PD6. Weekly updates for pip, github-actions, pre-commit, monthly for docker. adcp and googleads ignored per D16. * ci: add security.yml (zizmor + pip-audit + gitleaks + pinact + actionlint) Closes PD13. Per D35: gitleaks added as both pre-commit hook (staged files) and workflow job (full-history SARIF scan). zizmor gates on medium+ severity findings from day 1. * ci: add CodeQL in advisory mode (Path C, D10) Advisory for 2-week ramp (continue-on-error: true), then flip to gating. Uses security-extended queries, excludes alembic versions and test directories. * docs: add ADR-001 (uv.lock single source) and ADR-002 (solo-maintainer bypass) Documents the two architectural decisions locked in by this PR: pre-commit deps sourced from uv.lock only, and branch protection bypass for the solo maintainer (@ChrisHuie). * ci: SHA-pin all GitHub Actions + permissions:{} + persist-credentials:false All uses: refs converted to SHA@tag-comment format via pinact. permissions:{} added to test.yml (workflow-level deny-by-default). persist-credentials:false on all actions/checkout steps. * ci: add zizmor allowlist, ADR-003, and security PR title type - .github/zizmor.yml: allowlists ipr-agreement.yml and pr-title-check.yml for pull-request-target (ADR-003) and ipr-agreement.yml for excessive-permissions (CLA requires them) - ADR-003: documents pull_request_target trust boundary policy - pr-title-check.yml: add 'security' to allowed Conventional Commit types * fix: move [project.urls] after [project.optional-dependencies] in pyproject.toml TOML parsing bug: [project.urls] was placed before the dependencies key, causing uv to parse dependencies as project.urls.dependencies rather than project.dependencies. Moves [project.urls] to after all [project.*] inline data blocks. * ci: fix zizmor.yml allowlist rule names and add missing entries - pull-request-target -> dangerous-triggers (correct zizmor rule ID) - Add archived-uses entry for ipr-agreement.yml (contributor-assistant repo is archived, risk accepted) - Add excessive-permissions entries for release-please.yml and pr-title-check.yml with justification comments * ci: fix pinact installation in security.yml The install script URL at suzuki-shunsuke/pinact/main/scripts/ returns 404 for v3.x. Switch to direct binary download from GitHub releases (pinact_linux_amd64.tar.gz). * ci: fix zizmor.yml allowlist — correct rule names and file format - Use base filenames (not full paths) per zizmor config spec - dangerous-triggers: correct rule ID (was pull-request-target) - archived-uses: ipr-agreement.yml uses archived contributor-assistant - excessive-permissions: release-please.yml and pr-title-check.yml * docs: add inline explanations to dependabot, codeql, and pre-commit config - dependabot.yml: explain why googleads is pinned (GAM API version selected by library version) and why adcp is ignored until #1217 - codeql.yml: add step-by-step instructions for flipping from advisory to gating mode (~2 weeks after PR 1 merges) - .pre-commit-config.yaml: explain why SHA pins are used instead of tags (immutability, supply-chain security) and how to update them * docs: add ADR index and explain what Architecture Decision Records are - docs/decisions/README.md: new file explaining what ADRs are, why they exist, and how to add new ones. Includes index table of all ADRs. - docs/index.md: add ADR section so developers discovering the docs directory find the decisions/ folder and understand its purpose. * test: verify pre-commit hooks are running * fix(ci): suppress CVE-2026-46678 (pydantic-ai) pending coordinated upgrade pydantic-ai < 1.99.0 is vulnerable to CVE-2026-46678 (arbitrary code execution via unsafe deserialization, GHSA-cqp8-fcvh-x7r3). The fix requires pydantic-ai >= 1.99.0, which also pulls fastmcp >= 3.3.1. fastmcp 3.3.1 removed the top-level FastMCP re-export our codebase depends on, making this a coordinated migration tracked in issue #1234 (PR 2 scope). Suppress the advisory in both uv-secure (scripts/security-audit.sh) and pip-audit (security.yml) with a TODO(#1234-pr2) marker so the suppression is automatically visible when PR 2 is being developed. * fix(ci): address review feedback — checksum verification and shared vuln IDs Requested changes from @numarasSigmaSoftware review #4346272509: 1. Avoid unverified tool installs (security.yml): - pinact: download binary + checksums file separately, verify with sha256sum before extracting. Removes the bare 'curl | tar' pipe. - actionlint: same pattern — download binary + checksums file, verify sha256sum before extracting. Removes the pipe from main branch script (raw.githubusercontent.com/rhysd/actionlint/main/...). 2. Align ignored vulnerability IDs across audit gates: - Add scripts/security-ignored-vulns.sh as the single source of truth. Contains both GHSA and CVE identifiers for the same advisory with cross-reference comment (GHSA-cqp8-fcvh-x7r3 == CVE-2026-46678). - security-audit.sh (uv-secure): sources security-ignored-vulns.sh, uses UV_SECURE_IGNORED_VULNS variable. - security.yml (pip-audit): sources security-ignored-v.sh, uses PIP_AUDIT_IGNORED_VULNS variable. * fix(ci): suppress PYSEC-2026-161 (starlette) pending fastapi upgrade starlette 0.50.0 is vulnerable to PYSEC-2026-161 (fixed in 1.0.1). Upgrade blocked by fastapi 0.128.0 which constrains starlette < 1.0.0; fastmcp and mcp also pin starlette via transitive deps. Add PYSEC-2026-161 to scripts/security-ignored-vulns.sh with rationale and TODO(#1234-pr2) marker. Also document it in security-audit.sh. The coordinated fastapi + starlette upgrade is PR 2 scope. * fix(tests): track if_wholesale_feed_version as known schema-library mismatch The AdCP spec at adcontextprotocol.org/schemas/latest added the if_wholesale_feed_version field to get-products-request after the last successful CI run. The adcp library does not yet expose this field. Add to KNOWN_SCHEMA_LIBRARY_MISMATCHES so the alignment test records the drift without blocking CI, consistent with the existing pattern for if_catalog_version, if_pricing_version, and similar pre-flight fields. * fix(tests): restore xfail(strict=False) on test_offline_mode for spec drift The AdCP /schemas/latest/ spec updated again after #1336 removed the strict xfail. The minimal payload {products:[], cache_scope:public} now fails response schema validation because the live spec added new required fields not yet modelled in the adcp library (tracked in #1308). Restore the xfail with strict=False so that: - test FAILS (spec still ahead of library) => recorded as xfail, CI green - test PASSES (spec and library converge) => recorded as xpass, CI green Unlike the strict=True removed in #1336, strict=False tolerates both directions of drift without flipping CI red. * fix(ci): address review round 2 — ADR-001 divergence, pipefail, nits H4: Add FIXME(#1234-pr2) comment to mirrors-mypy block in .pre-commit-config.yaml explaining it violates ADR-001 and is scoped for migration in PR 2. Update ADR-001 status to 'Accepted — partially implemented' to align contract with current code. numarasSigmaSoftware: add set -euo pipefail to pip-audit shell block for consistency with pinact and actionlint blocks. S2: zizmor.yml archived-uses comment now cites #1234-pr6 instead of 'future PR'. S3: zizmor.yml excessive-permissions comments use #1234-pr3 convention. S4: dependabot.yml TODO(#1234) corrected to TODO(#1217) (the actual unblock issue). S5: security.yml adds inline comments on checksums-from-same-tag residual risk and references #1234-pr6 (cosign) as the planned closure. * fix(ci): add TODO(#1234-pr3) for gitleaks CI job to close bypass gap Pre-commit gitleaks hook can be bypassed with SKIP=gitleaks. Adding a CI-level gitleaks job is PR 3 scope (requires SARIF upload, baseline management, false positive handling). Comment makes the gap explicit and traceable. * fix(tests): remove stale __main__ call to deleted test_valid_get_products_response The function was removed in #1186 (PR #1334 notes) but the __main__ debug block still called it, causing NameError when running the file directly. Remove the dead call; all other __main__ references remain intact.
KonstantinMirin
added a commit
to KonstantinMirin/prebid-salesagent
that referenced
this pull request
May 25, 2026
…, prebid#1338, prebid#1337) Resolved 23 conflicts: - .beads/beads.db: taken ours (never touch beads) - Scripts/CI: taken theirs (security hardening, test-stack fixes) - creative_agent_registry.py: kept our stable adcp.types imports - delivery.py, media_buy_*.py: kept our DurationUnit/comment fixes - media_buy_update.py: merged imports from both sides - BDD conftest/steps: kept ours (all xfail graduation work) - tox.ini: merged env vars from both sides - Architecture guard: regenerated line-number allowlist Verified: 4640 unit tests pass, 1437 BDD tests pass, mypy clean.
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
2 participants
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.
PR #1334 added strict xfail markers asserting the get-products-request schema drift would fail; PR #1276 then added allowlist coverage (if_catalog_version, if_pricing_version in KNOWN_SCHEMA_LIBRARY_MISMATCHES; adcp_major_version in _VERSION_FIELDS). The combination made the tests xpass, breaking main CI.
Two markers removed:
Tracked: the underlying #1308 issue (adcp library doesn't model these fields natively) is still open — the allowlists are the workaround until adcp library upgrades.