Skip to content

feat: implement ADK wrapper functions for Agent Engine sessions#1252

Open
nohe427 wants to merge 1 commit intomainfrom
adk-wrapper
Open

feat: implement ADK wrapper functions for Agent Engine sessions#1252
nohe427 wants to merge 1 commit intomainfrom
adk-wrapper

Conversation

@nohe427
Copy link
Copy Markdown

@nohe427 nohe427 commented Mar 31, 2026

No description provided.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a 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 Firebase Cloud Functions wrapper proxy for Google Cloud's Agent Engine (ADK), providing secure, authenticated access to reasoning engine endpoints for client applications. The implementation includes session management, memory operations, and streaming query support. Key feedback includes correcting the Node.js runtime version in package.json, fixing a bug in adk.ts where streaming response data was incorrectly parsed as a string instead of a Protobuf value, and improving API error handling by replacing generic errors with structured HttpsError responses for unauthenticated requests.

"logs": "firebase functions:log"
},
"engines": {
"node": "24"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The Node.js version "24" is likely a typo or refers to a version not yet supported by Firebase Functions. It is recommended to use a stable LTS version like "22" or "20".

Suggested change
"node": "24"
"node": "22"

Comment on lines +53 to +57
const parsed = JSON.parse(response.data.toString('utf8'));
chunks.push(parsed);
if (parsed.content && parsed.content.parts) {
fullText += parsed.content.parts.map((p: any) => p.text || "").join("");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The response.data field in the aiplatform SDK's stream response is a google.protobuf.Value object, not a Buffer. Using JSON.parse(response.data.toString('utf8')) will fail. Use helpers.fromValue(response.data) to correctly parse the Protobuf value into a JavaScript object. Additionally, using optional chaining ensures the code is resilient to unexpected response structures.

Suggested change
const parsed = JSON.parse(response.data.toString('utf8'));
chunks.push(parsed);
if (parsed.content && parsed.content.parts) {
fullText += parsed.content.parts.map((p: any) => p.text || "").join("");
}
const parsed = helpers.fromValue(response.data) as any;
chunks.push(parsed);
if (parsed?.content?.parts) {
fullText += parsed.content.parts.map((p: any) => p.text || "").join("");
}

@@ -0,0 +1,19 @@
import { onCall } from "firebase-functions/v2/https";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Import HttpsError to provide structured error responses to the client. This should be applied to all endpoint files.

Suggested change
import { onCall } from "firebase-functions/v2/https";
import { onCall, HttpsError } from "firebase-functions/v2/https";

Comment on lines +14 to +16
if (!uid) {
throw new Error("Unauthorized");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use HttpsError with the unauthenticated code instead of a generic Error. Generic errors are returned to the client as INTERNAL (500), which hides the actual cause and is poor practice for API design. This pattern should be followed in all other endpoint files as well.

Suggested change
if (!uid) {
throw new Error("Unauthorized");
}
if (!uid) {
throw new HttpsError("unauthenticated", "The function must be called while authenticated.");
}

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.

1 participant