Skip to content

Commit 743b1a0

Browse files
committed
fix: address PR review feedback on retry-improvement
- plugin.py: drop redundant tool_input re-fetch (already bound at line 177) - plugin.py: document that invocation_state markers are intentionally per-invocation and do not need explicit cleanup - test_plugin_integration.py: hoist unittest.mock and PaymentError imports to module top, per project convention - test_plugin_integration.py: clarify TestPostPaymentFailureFlow docstring to flag it as mock-driven and credential-free
1 parent 1651332 commit 743b1a0

2 files changed

Lines changed: 12 additions & 16 deletions

File tree

src/bedrock_agentcore/payments/integrations/strands/plugin.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ def after_tool_call(self, event: AfterToolCallEvent) -> None:
218218
self._increment_payment_retry_count(event)
219219

220220
# Validate tool input before processing payment
221-
tool_input = event.tool_use.get("input", {})
222221
if not handler.validate_tool_input(tool_input):
223222
logger.error("Tool input validation failed, cannot apply payment header")
224223
self._store_payment_failure_state(event, Exception("Tool input validation failed"))
@@ -317,6 +316,11 @@ def _mark_successful_signing(self, event: AfterToolCallEvent) -> None:
317316
right before setting event.retry. If a subsequent 402 is received,
318317
_has_successful_signing will return True indicating the failure is server-side.
319318
319+
Note: payment_signed_*, payment_retry_count_*, and payment_failure_* keys are
320+
intentionally not cleared. invocation_state is scoped to a single agent
321+
invocation and is discarded by Strands when the invocation ends, so these
322+
per-tool-use markers do not accumulate across invocations.
323+
320324
Args:
321325
event: The after tool call event
322326
"""

tests_integ/payments/integrations/strands/test_plugin_integration.py

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import os
99
import uuid
1010
from typing import Any
11+
from unittest.mock import MagicMock, patch
1112

1213
import pytest
1314
from strands import Agent
@@ -16,6 +17,7 @@
1617
from bedrock_agentcore.payments.integrations.config import AgentCorePaymentsPluginConfig
1718
from bedrock_agentcore.payments.integrations.strands.plugin import AgentCorePaymentsPlugin
1819
from bedrock_agentcore.payments.manager import (
20+
PaymentError,
1921
PaymentInstrumentConfigurationRequired,
2022
PaymentManager,
2123
PaymentSessionConfigurationRequired,
@@ -714,10 +716,12 @@ def test_plugin_with_agent_name_initializes_payment_manager(self):
714716

715717
@pytest.mark.integration
716718
class TestPostPaymentFailureFlow:
717-
"""Hook-level integration tests for post-payment failure behavior.
719+
"""Hook-flow scenarios for post-payment failure behavior.
718720
719-
Exercises the plugin's after_tool_call hook directly to verify the
720-
retry logic end-to-end without requiring an LLM.
721+
These are mock-driven tests that exercise the plugin's after_tool_call hook
722+
directly. They fully mock PaymentManager, do not hit AWS, and require no
723+
credentials — they live here alongside the real-AWS integration tests only
724+
because they cover end-to-end hook flow rather than a single helper method.
721725
"""
722726

723727
@classmethod
@@ -728,8 +732,6 @@ def setup_class(cls):
728732

729733
def _make_402_event(self, tool_use_id, body, invocation_state=None, tool_input=None):
730734
"""Create a mock AfterToolCallEvent with a 402 PAYMENT_REQUIRED result."""
731-
from unittest.mock import MagicMock
732-
733735
payment_required = {
734736
"statusCode": 402,
735737
"headers": {"content-type": "application/json"},
@@ -760,8 +762,6 @@ def test_full_flow_402_sign_retry_402_stops(self):
760762
1. First after_tool_call with 402 → plugin signs and sets retry=True
761763
2. Second after_tool_call with 402 (server rejection) → plugin stops, no retry
762764
"""
763-
from unittest.mock import MagicMock, patch
764-
765765
mock_pm = MagicMock()
766766
mock_pm.generate_payment_header.return_value = {"X-PAYMENT": "signed-proof-base64"}
767767

@@ -825,10 +825,6 @@ def test_signing_failure_uses_retry_counter(self):
825825
Simulates multiple after_tool_call invocations where signing always fails.
826826
Verifies that MAX_PAYMENT_RETRIES is respected.
827827
"""
828-
from unittest.mock import MagicMock, patch
829-
830-
from bedrock_agentcore.payments.manager import PaymentError
831-
832828
mock_pm = MagicMock()
833829
mock_pm.generate_payment_header.side_effect = PaymentError("Signing service unavailable")
834830

@@ -891,8 +887,6 @@ def test_successful_sign_then_success_response(self):
891887
After successful signing and retry, if the tool returns a non-402 response,
892888
the plugin should not interfere.
893889
"""
894-
from unittest.mock import MagicMock, patch
895-
896890
mock_pm = MagicMock()
897891
mock_pm.generate_payment_header.return_value = {"X-PAYMENT": "valid-proof"}
898892

@@ -951,8 +945,6 @@ def test_signing_state_independent_per_tool_use(self):
951945
- tool-A signed successfully → subsequent 402 stops
952946
- tool-B never signed → 402 still attempts signing
953947
"""
954-
from unittest.mock import MagicMock, patch
955-
956948
mock_pm = MagicMock()
957949
mock_pm.generate_payment_header.return_value = {"X-PAYMENT": "signed"}
958950

0 commit comments

Comments
 (0)