-
Notifications
You must be signed in to change notification settings - Fork 107
feat: add A2a agent quickstart sample #277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add A2a agent quickstart sample #277
Conversation
Summary of ChangesHello @PierrickVoulet, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a new quickstart sample designed to illustrate the integration of a Google Chat bot with an Agent-to-Agent (A2A) AI agent, leveraging the Vertex AI Agent Engine. The sample provides a comprehensive framework for processing user interactions from Google Chat, orchestrating communication with the AI agent, and rendering rich, interactive responses back to the user, including support for file attachments and structured UI elements. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a Google Apps Script project that integrates a Google Chat application with an A2A-wrapped AI agent hosted in Vertex AI Agent Engine. The changes include core logic for sending messages to the AI agent, polling its task status, and processing its responses, along with functions for handling Google Chat events, managing service account credentials, and configuring environment variables. Review comments highlight several critical areas for improvement, including adding robust error handling for all external API calls (e.g., UrlFetchApp.fetch, Chat API calls) by checking HTTP response codes and using try...catch blocks, addressing potential script timeouts by making the agent processing asynchronous and implementing a timeout mechanism for the task polling loop, correcting a bug in the debug mode parsing logic, improving the robustness of location extraction from resource names, and refining a regular expression used for removing references from agent responses to prevent unintended truncation.
| const sendResponseContentText = UrlFetchApp.fetch( | ||
| `https://${getLocation()}-aiplatform.googleapis.com/v1beta1/${getReasoningEngine()}/a2a/v1/message:send`, | ||
| { | ||
| method: 'post', | ||
| headers: { 'Authorization': `Bearer ${getCredentials().getAccessToken()}` }, | ||
| contentType: 'application/json', | ||
| payload: JSON.stringify({ | ||
| "message": { | ||
| "messageId": Utilities.getUuid(), | ||
| "role": "1", | ||
| "content": extractContentFromInput(input) | ||
| } | ||
| }), | ||
| muteHttpExceptions: true | ||
| } | ||
| ).getContentText(); | ||
| if (isInDebugMode()) { | ||
| console.log("Send response: " + sendResponseContentText); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UrlFetchApp.fetch call has muteHttpExceptions: true, but there is no subsequent check on the HTTP response code. If the API call fails, getContentText() will return an error payload, and JSON.parse() on line 43 will likely throw an unhandled exception, crashing the script. You should check the response code and handle errors gracefully before parsing the response.
const sendHttpResponse = UrlFetchApp.fetch(
`https://${getLocation()}-aiplatform.googleapis.com/v1beta1/${getReasoningEngine()}/a2a/v1/message:send`,
{
method: 'post',
headers: { 'Authorization': `Bearer ${getCredentials().getAccessToken()}` },
contentType: 'application/json',
payload: JSON.stringify({
"message": {
"messageId": Utilities.getUuid(),
"role": "ROLE_USER",
"content": extractContentFromInput(input)
}
}),
muteHttpExceptions: true
}
);
const sendResponseContentText = sendHttpResponse.getContentText();
if (sendHttpResponse.getResponseCode() >= 400) {
console.error(`Error sending message to agent: ${sendResponseContentText}`);
// TODO: Inform the user about the error.
return;
}
if (isInDebugMode()) {
console.log("Send response: " + sendResponseContentText);
}|
|
||
| // Retrieve the ID of the resulting task | ||
| const sendResponse = JSON.parse(sendResponseContentText); | ||
| taskId = sendResponse.task.id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable taskId is assigned without being declared using let, const, or var. This creates an implicit global variable, which is a dangerous practice in JavaScript and Apps Script as it can lead to unpredictable behavior and makes the code harder to debug. Please declare taskId with const.
const taskId = sendResponse.task.id;| const taskResponseContentText = UrlFetchApp.fetch( | ||
| `https://${getLocation()}-aiplatform.googleapis.com/v1beta1/${getReasoningEngine()}/a2a/v1/tasks/${taskId}?history_length=1`, | ||
| { | ||
| method: 'get', | ||
| headers: { 'Authorization': `Bearer ${getCredentials().getAccessToken()}` }, | ||
| contentType: 'application/json', | ||
| muteHttpExceptions: true | ||
| } | ||
| ).getContentText(); | ||
| if (isInDebugMode()) { | ||
| console.log("Get task response: " + taskResponseContentText); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This UrlFetchApp.fetch call to get the task status has muteHttpExceptions: true but lacks error handling. If this API call fails, JSON.parse on line 64 will likely throw an unhandled exception. You should check the response code and handle any potential errors, for instance by breaking the polling loop.
const taskHttpResponse = UrlFetchApp.fetch(
`https://${getLocation()}-aiplatform.googleapis.com/v1beta1/${getReasoningEngine()}/a2a/v1/tasks/${taskId}?history_length=1`,
{
method: 'get',
headers: { 'Authorization': `Bearer ${getCredentials().getAccessToken()}` },
contentType: 'application/json',
muteHttpExceptions: true
}
);
const taskResponseContentText = taskHttpResponse.getContentText();
if (taskHttpResponse.getResponseCode() >= 400) {
console.error(`Error fetching task status: ${taskResponseContentText}`);
break;
}
if (isInDebugMode()) {
console.log("Get task response: " + taskResponseContentText);
}| do { | ||
| Utilities.sleep(1000); // Wait a bit before polling | ||
| const taskResponseContentText = UrlFetchApp.fetch( | ||
| `https://${getLocation()}-aiplatform.googleapis.com/v1beta1/${getReasoningEngine()}/a2a/v1/tasks/${taskId}?history_length=1`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The history_length=1 query parameter will cause the API to return at most one message from the task's history. However, the logic in this loop is designed to process multiple new messages from sub-agents. If the agent produces multiple messages between polling intervals, this implementation will only ever see the most recent one, causing previous messages to be missed. This will lead to incomplete or incorrect agent responses. To fetch all new messages, you should remove this parameter to use the API's default.
`https://${getLocation()}-aiplatform.googleapis.com/v1beta1/${getReasoningEngine()}/a2a/v1/tasks/${taskId}`,| setChatConfig(chatEvent.messagePayload.space.name); | ||
|
|
||
| // Request AI agent to answer the message | ||
| requestAgent(chatEvent.user.name, chatEvent.messagePayload.message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onMessage function directly calls requestAgent, which is a long-running synchronous operation due to its polling mechanism. Google Chat webhooks require a response within 30 seconds. If requestAgent takes longer, the request will time out, and the user will see an "App is not responding" error. To fix this, you should make the agent processing asynchronous. The onMessage function should return immediately after triggering the asynchronous task, for example by using ScriptApp.newTrigger(). The triggered function can then perform the long-running work and post messages back to the chat space.
| // Creates widgets for the markdown text and grounding response. | ||
| function createMarkdownAndGroundingWidgets(text, groundingMetadata) { | ||
| // Remove the references from text | ||
| let widgets = createMarkdownWidgets(text.replace(/^\s*```(json)?[\s\S]*?```\s*/i, '').replace(/Reference(s)?:[\s\S]*/i, '')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regular expression .replace(/Reference(s)?:[\s\S]*/i, '') is too greedy. It will remove everything from the first occurrence of "Reference:" or "References:" to the end of the string. If this pattern appears unexpectedly in the middle of the agent's response, it will cause the message to be truncated. To make this safer, consider anchoring the pattern to only match if it appears at the end of the text, for example, as its own paragraph.
let widgets = createMarkdownWidgets(text.replace(/^\s*```(json)?[\s\S]*?```\s*/i, '').replace(/\n\nReference(s)?:[\s\S]*$/i, ''))| const response = UrlFetchApp.fetch( | ||
| `https://chat.googleapis.com/v1/media/${attachmentName}?alt=media`, | ||
| { | ||
| method: 'get', | ||
| headers: { 'Authorization': `Bearer ${getCredentials().getAccessToken()}` }, | ||
| muteHttpExceptions: true | ||
| } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UrlFetchApp.fetch call to download an attachment has muteHttpExceptions: true but does not check the response code. If the media download fails, response.getContent() could return an error message or be empty, which would then be passed to Utilities.base64Encode(). This could lead to corrupted data or script errors. You should check the HTTP response code and handle failures appropriately.
const response = UrlFetchApp.fetch(
`https://chat.googleapis.com/v1/media/${attachmentName}?alt=media`,
{
method: 'get',
headers: { 'Authorization': `Bearer ${getCredentials().getAccessToken()}` },
muteHttpExceptions: true
}
);
if (response.getResponseCode() >= 400) {
console.error('Failed to download attachment %s: %s', attachmentName, response.getContentText());
throw new Error('Attachment download failed.');
}| function createMessage(message) { | ||
| const spaceName = getConfiguredChat(); | ||
| console.log(`Creating message in space ${spaceName}...`); | ||
| return Chat.Spaces.Messages.create( | ||
| message, | ||
| spaceName, | ||
| {}, | ||
| {'Authorization': `Bearer ${getCredentials().getAccessToken()}`} | ||
| ).name; | ||
| } | ||
|
|
||
| // Updates a Chat message in the configured space. | ||
| function updateMessage(name, message) { | ||
| console.log(`Updating message ${name}...`); | ||
| Chat.Spaces.Messages.patch( | ||
| message, | ||
| name, | ||
| { updateMask: "*" }, | ||
| {'Authorization': `Bearer ${getCredentials().getAccessToken()}`} | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calls to Chat.Spaces.Messages.create and Chat.Spaces.Messages.patch do not have any error handling. If an API call fails (e.g., due to permission issues or an invalid message format), it will throw an unhandled exception and terminate the script. It's important to wrap these calls in try...catch blocks to handle potential API errors gracefully and log them for debugging.
| function getLocation() { | ||
| const parts = REASONING_ENGINE_RESOURCE_NAME.split('/'); | ||
| const locationIndex = parts.indexOf('locations') + 1; | ||
| return parts[locationIndex]; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic to extract the location from REASONING_ENGINE_RESOURCE_NAME is brittle. It relies on splitting the string by / and assuming locations is always present and followed by the location string. If the resource name format is invalid, this could return an incorrect value (like the project ID) and cause API calls to fail. A regular expression would be a more robust way to extract the location.
| function getLocation() { | |
| const parts = REASONING_ENGINE_RESOURCE_NAME.split('/'); | |
| const locationIndex = parts.indexOf('locations') + 1; | |
| return parts[locationIndex]; | |
| } | |
| function getLocation() { | |
| const match = REASONING_ENGINE_RESOURCE_NAME.match(/locations\/([^\/]+)/); | |
| if (!match || !match[1]) { | |
| throw new Error('Could not parse location from REASONING_ENGINE_RESOURCE_NAME: ' + REASONING_ENGINE_RESOURCE_NAME); | |
| } | |
| return match[1]; | |
| } |
| const DEBUG = parseInt(PropertiesService.getScriptProperties().getProperty('DEBUG')) || 0; | ||
|
|
||
| // Returns whether the application is running in debug mode. | ||
| function isInDebugMode() { | ||
| return DEBUG == 1 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for parsing the DEBUG script property is buggy. parseInt() returns NaN for non-numeric strings like "true", and NaN || 0 evaluates to 0. This means isInDebugMode() will incorrectly return false if the DEBUG property is set to "true". To correctly handle boolean-like strings (e.g., "true") and the number 1, you should check for these values explicitly.
const DEBUG_PROPERTY = PropertiesService.getScriptProperties().getProperty('DEBUG');
const DEBUG = DEBUG_PROPERTY === '1' || (DEBUG_PROPERTY && DEBUG_PROPERTY.toLowerCase() === 'true');
// Returns whether the application is running in debug mode.
function isInDebugMode() {
return DEBUG;
}
No description provided.