Skip to content

Add Gemini buddy example#190

Open
kaisoz wants to merge 5 commits into
google:mainfrom
kaisoz:gemini-buddy-example
Open

Add Gemini buddy example#190
kaisoz wants to merge 5 commits into
google:mainfrom
kaisoz:gemini-buddy-example

Conversation

@kaisoz

@kaisoz kaisoz commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 new development example, gemini-buddy-mcp, which implements an MCP service wrapping the Gemini CLI to maintain multi-turn conversation state server-side. The feedback highlights several critical areas for improvement: resolving potential character corruption by explicitly setting UTF-8 encoding on child process streams, avoiding the security risk of hardcoding API keys in the Dockerfile, sanitizing user messages containing newlines to preserve the transcript structure, and addressing potential race conditions from concurrent requests on the in-memory session store.

Comment on lines +35 to +38
let stdout = "";
let stderr = "";
child.stdout.on("data", (d) => { stdout += d; });
child.stderr.on("data", (d) => { stderr += d; });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

When reading from child.stdout and child.stderr, the data chunks are emitted as Buffer objects. Concatenating them directly to a string (stdout += d) implicitly converts each buffer chunk to a string. If a multi-byte character (such as an emoji or non-ASCII character) is split across chunk boundaries, this implicit conversion will result in corrupted characters (e.g., ``).

To prevent this, set the encoding of the streams to 'utf8' before listening to the 'data' event, which ensures that multi-byte characters are decoded correctly across chunk boundaries.

Suggested change
let stdout = "";
let stderr = "";
child.stdout.on("data", (d) => { stdout += d; });
child.stderr.on("data", (d) => { stderr += d; });
let stdout = "";
let stderr = "";
child.stdout.setEncoding("utf8");
child.stderr.setEncoding("utf8");
child.stdout.on("data", (d) => { stdout += d; });
child.stderr.on("data", (d) => { stderr += d; });

Comment on lines +10 to +13
# Dev-only: replace <API_KEY> with a Google AI Studio free-tier key before `make kind`.
# Keep the edit local so it is never committed:
# git update-index --skip-worktree development/examples/gemini-buddy-mcp/Dockerfile
ENV GEMINI_API_KEY=<API_KEY>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

Hardcoding API keys in the Dockerfile (even with instructions to use git update-index --skip-worktree) is a security risk. The key can easily be baked into the Docker image layers, making it retrievable by anyone with access to the image, and it is prone to accidental commits if the local git configuration is reset or cloned elsewhere.

Instead, GEMINI_API_KEY should be passed as a runtime environment variable to the container (e.g., via Kubernetes secrets, Docker run flags, or local compose/mesh configuration) and not defined with a default placeholder in the Dockerfile.

Comment on lines +27 to +29
function render(turns) {
return turns.map((t) => `${t.role === "user" ? "User" : "Buddy"}: ${t.text}`).join("\n");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If a user message contains newlines, the rendered transcript will contain lines that are not prefixed with User: or Buddy:. This violates the assumption in the system prompt (You'll get the conversation so far on stdin, lines prefixed 'User:' and 'Buddy:') and can confuse the model.

Consider sanitizing or replacing newlines in the message text to keep the transcript structure clean and predictable.

function render(turns) {
  return turns.map((t) => {
    const role = t.role === "user" ? "User" : "Buddy";
    const cleanText = t.text.replace(/\\r?\\n/g, " ");
    return `${role}: ${cleanText}`;
  }).join("\\n");
}

Comment on lines +64 to +76
async ({ message, session_id }) => {
const id = session_id ?? "default";
const turns = transcript(id);
turns.push({ role: "user", text: message });
try {
const reply = await runGemini(render(turns));
turns.push({ role: "assistant", text: reply });
return { content: [{ type: "text", text: reply }] };
} catch (err) {
turns.pop(); // drop the turn we couldn't answer so it can be retried
return { content: [{ type: "text", text: String(err?.message ?? err) }], isError: true };
}
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since the server handles requests asynchronously and stores session history in a shared in-memory sessions Map, concurrent requests for the same session_id can lead to race conditions. If two chat requests for the same session are processed concurrently, both will read the same initial transcript, append their respective messages, and call runGemini in parallel. This can result in corrupted history, duplicate turns, or out-of-order messages.

While this is a development example, it is worth noting that in a production environment, you should serialize requests per session (e.g., using a promise chain or queue per session ID) or use a persistent, transactional database.

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