Skip to content

Conversation

@bokelley
Copy link
Contributor

Summary

Fixes critical bug where update_media_buy called get_current_tenant() before establishing tenant context via authentication, causing "No tenant context set" errors. Adds three layers of protection to prevent regression: enhanced error messages, 7 unit tests, and pre-commit hook.

Changes

  • update_media_buy: Move auth call before tenant context access
  • get_current_tenant(): Enhanced error message with caller information
  • test_tenant_context_ordering.py: 7 comprehensive tests for regression prevention
  • check_tenant_context_order.py: Pre-commit hook to detect similar patterns

Test Results

✅ 988 unit tests passed
✅ 35 integration tests passed
✅ 15 integration_v2 tests passed
✅ All pre-commit hooks passed

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

bokelley and others added 2 commits November 21, 2025 19:10
Fix critical bug where update_media_buy called get_current_tenant() before
establishing tenant context via authentication. This caused "No tenant context
set" errors. Also add comprehensive protection: enhanced error messages with
caller info, 7 unit tests for regression prevention, and pre-commit hook to
detect similar bugs in future tool implementations.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Update test to use valid authentication fixture instead of invalid mock token.
With the auth ordering fix, authentication now happens before buyer_ref lookup,
so the test needs a real principal token to pass auth before testing the
buyer_ref lookup logic.
@bokelley bokelley merged commit 2c2d9b1 into main Nov 22, 2025
8 checks passed
EmmaLouise2018 added a commit that referenced this pull request Nov 24, 2025
These 7 tests depend on the external creative agent at
https://creative.adcontextprotocol.org being available in CI, which is not
guaranteed. Skipping them to unblock the A2A parameter priority PR.

Tests skipped:
- test_gam_cpm_guaranteed_creates_standard_line_item
- test_gam_cpc_creates_price_priority_line_item_with_clicks_goal
- test_gam_vcpm_creates_standard_line_item_with_viewable_impressions
- test_gam_flat_rate_calculates_cpd_correctly
- test_gam_multi_package_mixed_pricing_models
- test_gam_accepts_cpm_pricing_model
- test_gam_accepts_cpm_from_multi_pricing_product

Tracked in: #773
danf-newton pushed a commit to Newton-Research-Inc/salesagent that referenced this pull request Nov 24, 2025
…otocol#773)

* fix: Correct tenant context ordering in update_media_buy

Fix critical bug where update_media_buy called get_current_tenant() before
establishing tenant context via authentication. This caused "No tenant context
set" errors. Also add comprehensive protection: enhanced error messages with
caller info, 7 unit tests for regression prevention, and pre-commit hook to
detect similar bugs in future tool implementations.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>

* test: Fix test_update_media_buy_requires_media_buy_id for auth ordering

Update test to use valid authentication fixture instead of invalid mock token.
With the auth ordering fix, authentication now happens before buyer_ref lookup,
so the test needs a real principal token to pass auth before testing the
buyer_ref lookup logic.

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

2 participants