fix: address review feedback

- Preserve interrupt behavior: post-payment failure stores state and
  triggers interrupt so downstream consumers are notified
- Remove dead code: _extract_payment_error_message (unreachable)
- Remove TestExtractPaymentErrorMessage (tested removed method)
- Clarify comment on retry counter increment
- Add test: signing flag takes precedence over retry limit
- Remove emojis from new integration test logger output

Author: Raju Ansari

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

@@ -198,20 +198,23 @@ def after_tool_call(self, event: AfterToolCallEvent) -> None:
 
             # If we previously signed successfully and still got a 402, the server
             # rejected the payment for a non-retryable reason (e.g., insufficient balance).
-            # Stop processing — the existing 402 result is already structured for the LLM.
+            # Do not retry — store failure state so the agent is notified via interrupt.
             if self._has_successful_signing(event):
+                error_msg = body.get("error", "unknown error") if body and isinstance(body, dict) else "unknown error"
                 logger.warning(
-                    "Received 402 after successful signing for tool %s — post-payment failure, not retrying",
+                    "Received 402 after successful signing for tool %s — post-payment failure: %s",
                     event.tool_use.get("name", "unknown"),
+                    error_msg,
                 )
+                self._store_payment_failure_state(event, PaymentError(f"Payment rejected after signing: {error_msg}"))
                 return
 
             # Check if signing retry limit has been reached
             if self._check_payment_retry_limit(event):
                 logger.warning("Payment signing retry limit reached. Processing skipped.")
                 return
 
-            # Increment retry count for signing attempts
+            # Increment before attempt so limit is enforced even on exception
             self._increment_payment_retry_count(event)
 
             # Validate tool input before processing payment
@@ -321,22 +324,6 @@ def _mark_successful_signing(self, event: AfterToolCallEvent) -> None:
         signed_key = f"payment_signed_{tool_use_id}"
         event.invocation_state[signed_key] = True
 
-    @staticmethod
-    def _extract_payment_error_message(body: Optional[Dict[str, Any]]) -> str:
-        """Extract a human-readable error message from a 402 response body.
-
-        Args:
-            body: The extracted response body (may be None)
-
-        Returns:
-            Error message string, or "unknown error" if not extractable
-        """
-        if body and isinstance(body, dict):
-            error = body.get("error")
-            if isinstance(error, str) and error:
-                return error
-        return "unknown error"
-
     def _store_payment_failure_state(self, event: AfterToolCallEvent, exception: Exception) -> None:
         """Store payment failure information in invocation state for agent to handle.
 

tests/bedrock_agentcore/payments/integrations/strands/test_plugin.py

@@ -1376,34 +1376,6 @@ def test_signing_state_is_per_tool_use(self):
         assert plugin._has_successful_signing(event2) is False
 
 
-class TestExtractPaymentErrorMessage:
-    """Tests for _extract_payment_error_message static method."""
-
-    def test_extracts_error_from_body(self):
-        """Test extracting error message from body dict."""
-        body = {"error": "invalid_exact_evm_insufficient_balance"}
-        assert AgentCorePaymentsPlugin._extract_payment_error_message(body) == "invalid_exact_evm_insufficient_balance"
-
-    def test_returns_unknown_for_none_body(self):
-        """Test returns 'unknown error' when body is None."""
-        assert AgentCorePaymentsPlugin._extract_payment_error_message(None) == "unknown error"
-
-    def test_returns_unknown_for_empty_error(self):
-        """Test returns 'unknown error' when error is empty string."""
-        body = {"error": ""}
-        assert AgentCorePaymentsPlugin._extract_payment_error_message(body) == "unknown error"
-
-    def test_returns_unknown_for_missing_error_key(self):
-        """Test returns 'unknown error' when error key is missing."""
-        body = {"statusCode": 402}
-        assert AgentCorePaymentsPlugin._extract_payment_error_message(body) == "unknown error"
-
-    def test_returns_unknown_for_non_string_error(self):
-        """Test returns 'unknown error' when error is not a string."""
-        body = {"error": 42}
-        assert AgentCorePaymentsPlugin._extract_payment_error_message(body) == "unknown error"
-
-
 class TestAfterToolCallPostPaymentFailure:
     """Tests for the post-payment failure path in after_tool_call."""
 
@@ -1443,12 +1415,13 @@ def test_post_payment_failure_does_not_retry_after_successful_signing(self, mock
 
         # Should NOT retry
         assert event.retry is False
-        # Should NOT store failure state (no interrupt cycle)
-        assert "payment_failure_tool-123" not in event.invocation_state
+        # Should store failure state so agent is notified via interrupt
+        assert "payment_failure_tool-123" in event.invocation_state
+        failure = event.invocation_state["payment_failure_tool-123"]
+        assert "Payment rejected after signing" in failure["exceptionMessage"]
+        assert "invalid_exact_evm_insufficient_balance" in failure["exceptionMessage"]
         # Should NOT have called generate_payment_header
         mock_pm_instance.generate_payment_header.assert_not_called()
-        # Result should remain unchanged (raw 402 for LLM to reason about)
-        assert event.result == original_result
 
     @patch("bedrock_agentcore.payments.integrations.strands.plugin.PaymentManager")
     def test_first_402_without_prior_signing_proceeds_normally(self, mock_payment_manager_class):
@@ -1558,6 +1531,47 @@ def test_signing_retry_limit_stops_processing(self, mock_payment_manager_class):
         assert event.retry is False
 
 
+class TestSigningFlagTakesPrecedenceOverRetryLimit:
+    """Tests that _has_successful_signing check takes precedence over retry limit."""
+
+    @patch("bedrock_agentcore.payments.integrations.strands.plugin.PaymentManager")
+    def test_successful_signing_stops_even_when_retry_limit_not_reached(self, mock_payment_manager_class):
+        """Test that post-payment failure stops processing regardless of retry counter state.
+
+        If signing succeeded (payment_signed_ flag is True), a subsequent 402 should
+        stop immediately — even if payment_retry_count is below MAX_PAYMENT_RETRIES.
+        The signing flag takes precedence over the retry counter.
+        """
+        config = AgentCorePaymentsPluginConfig(
+            payment_manager_arn="arn:aws:bedrock-agentcore:us-west-2:123456789012:payment-manager/test",
+            user_id="test-user",
+            payment_instrument_id="payment-instrument-123",
+            payment_session_id="payment-session-456",
+        )
+
+        mock_pm_instance = MagicMock()
+        plugin = AgentCorePaymentsPlugin(config=config)
+        plugin.payment_manager = mock_pm_instance
+
+        # Signing succeeded but retry counter is only at 1 (below MAX_PAYMENT_RETRIES=3)
+        event, _ = _create_event_with_agent(
+            {
+                "result": [{"text": f"PAYMENT_REQUIRED: {json.dumps({'statusCode': 402, 'headers': {}, 'body': {}})}"}],
+                "tool_use": {"name": "http_request", "toolUseId": "tool-123", "input": {"headers": {}}},
+                "invocation_state": {"payment_signed_tool-123": True, "payment_retry_count_tool-123": 1},
+                "retry": False,
+            }
+        )
+
+        plugin.after_tool_call(event)
+
+        # Should NOT retry — signing flag takes precedence
+        assert event.retry is False
+        mock_pm_instance.generate_payment_header.assert_not_called()
+        # Should store failure state for interrupt notification
+        assert "payment_failure_tool-123" in event.invocation_state
+
+
 class TestAfterToolCallAutoPaymentDisabled:
     """Tests for auto_payment=False in after_tool_call."""
 

tests_integ/payments/integrations/strands/test_plugin_integration.py

@@ -797,7 +797,7 @@ def test_full_flow_402_sign_retry_402_stops(self):
         mock_pm.generate_payment_header.assert_called_once()
         assert "X-PAYMENT" in tool_input["headers"]
         assert invocation_state.get("payment_signed_tool-abc") is True
-        logger.info("✓ Step 1: First 402 → signed and retry=True")
+        logger.info("Step 1: First 402 - signed and retry=True")
 
         # Step 2: Second 402 — server rejected (insufficient balance)
         event2 = self._make_402_event(
@@ -813,9 +813,11 @@ def test_full_flow_402_sign_retry_402_stops(self):
         assert event2.retry is False
         # generate_payment_header should still only have been called once (from step 1)
         assert mock_pm.generate_payment_header.call_count == 1
-        # Should NOT store failure state (no interrupt cycle)
-        assert "payment_failure_tool-abc" not in invocation_state
-        logger.info("✓ Step 2: Second 402 after signing → stopped, no retry")
+        # Should store failure state so agent is notified via interrupt
+        assert "payment_failure_tool-abc" in invocation_state
+        failure = invocation_state["payment_failure_tool-abc"]
+        assert "Payment rejected after signing" in failure["exceptionMessage"]
+        logger.info("Step 2: Second 402 after signing - stopped, failure stored for interrupt")
 
     def test_signing_failure_uses_retry_counter(self):
         """Test that signing failures increment retry counter and stop at limit.