Add PicoD Design #29
Add PicoD Design #29
Conversation
volcano-sh-bot
added
kind/documentation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @VanderChen! It looks like this is your first PR to volcano-sh/agentcube 🎉 |
Summary of ChangesHello @VanderChen, 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 introduces the comprehensive design for PicoD (Pico Daemon), a new lightweight, HTTP-based service daemon intended to replace the existing SSH-based sandbox management in AgentCube. The design focuses on providing core sandbox functionalities like code execution and file transfer through a gRPC interface, emphasizing minimal overhead, stateless operation, and robust token-based authentication. It details the architecture, security considerations, and integration with the Python SDK, aiming to improve efficiency and customization for AI agent interactions within sandboxed environments. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive design document for PicoD, a new gRPC-based daemon for sandbox management. The design is well-structured, detailed, and covers key aspects like architecture, authentication, and API definitions. I've provided a few comments to address minor inconsistencies, clarify some implementation details in the SDK, and define a missing data type in the protobuf specification. Overall, this is an excellent design proposal.
docs/design/PicoD-Design.md
Outdated
|
|
||
| **FilesystemService** | ||
| - `ReadFile(path) → stream bytes`: Download file content (replaces `download_file()`) | ||
| - `WriteFile(path, stream bytes) → void`: Upload file content (replaces `write_file()` and `upload_file()`) |
There was a problem hiding this comment.
There's an inconsistency in the documented return type for the WriteFile RPC. Here it is listed as returning void, but the protobuf definition on line 349 specifies it returns WriteFileResponse. For clarity and consistency, please update this line to match the protobuf definition.
| - `WriteFile(path, stream bytes) → void`: Upload file content (replaces `write_file()` and `upload_file()`) | |
| - `WriteFile(path, stream bytes) → WriteFileResponse`: Upload file content (replaces `write_file()` and `upload_file()`) |
docs/design/PicoD-Design.md
Outdated
| } | ||
|
|
||
| message WriteFileResponse { | ||
| EntryInfo entry = 1; |
There was a problem hiding this comment.
The WriteFileResponse message references a type EntryInfo, which is also used in ReadFileMetadata on line 385. However, the EntryInfo message is not defined in the protobuf specifications. Please add the definition for EntryInfo to clarify what file details (e.g., path, size, permissions, timestamps) are returned.
docs/design/PicoD-Design.md
Outdated
| The Python SDK provides a simple interface for interacting with PicoD: | ||
|
|
||
| ```python | ||
| class PicoDClient: |
There was a problem hiding this comment.
The PicoDClient code snippet uses type hints like List, Dict, and Optional (in methods on lines 406, 412, 415, etc.), but the corresponding imports from the typing module are missing. For correctness and to provide a complete example for implementers, please add from typing import Dict, List, Optional at the beginning of this code block.
docs/design/PicoD-Design.md
Outdated
| def execute_commands(self, commands: List[str]) -> Dict[str, str]: | ||
| """Execute multiple commands""" |
There was a problem hiding this comment.
The Python SDK proposes an execute_commands method for batch execution. However, the gRPC service definition only includes a unary Execute RPC for a single command. To avoid ambiguity, please clarify if execute_commands is a client-side convenience function that calls Execute in a loop, or if a more efficient batch/streaming RPC is planned for this functionality. If it's a client-side loop, it might be worth noting the potential performance implications for a large number of commands.
VanderChen
force-pushed
the
picod-design
branch
from
06e8e06 to
e7da91b
Compare
docs/design/PicoD-Design.md
Outdated
|
|
||
| ### Design Goals | ||
|
|
||
| PicoD is designed as a **stateless daemon** that processes individual gRPC requests independently: |
There was a problem hiding this comment.
In L12, you said it is HTTP based
docs/design/PicoD-Design.md
Outdated
| from agentcube import Sandbox | ||
|
|
||
| # Create a sandbox instance | ||
| sandbox = Sandbox(ttl=3600, image="python:3.11-slim") |
There was a problem hiding this comment.
This pr https://github.com/volcano-sh/agentcube/pull/19/files is abstracting a codeInterpreterClient class
docs/design/PicoD-Design.md
Outdated
|
|
||
| PicoD checks token sources in the following order: | ||
| 1. `--access-token` command-line flag (highest priority) | ||
| 2. `PICOD_ACCESS_TOKEN` environment variable |
There was a problem hiding this comment.
We cannot use env to inject token
docs/design/PicoD-Design.md
Outdated
|
|
||
| message ExecuteRequest { | ||
| string command = 1; // Full command string to execute | ||
| optional float timeout = 2; // Execution timeout in seconds (default: 30) |
There was a problem hiding this comment.
we need some other attributes like setting env
docs/design/PicoD-Design.md
Outdated
| } | ||
|
|
||
| message WriteFileResponse { | ||
| EntryInfo entry = 1; |
There was a problem hiding this comment.
what does EntryInfo look like
|
cc @YaoZengzeng |
| #### System Architecture | ||
|
|
||
| ```mermaid | ||
| graph TB |
There was a problem hiding this comment.
The agentcube-apiserver should be shown in the architecture? Although it only performs transport forwarding.
| #### Authentication Flow | ||
|
|
||
| ```mermaid | ||
| sequenceDiagram |
docs/design/PicoD-Design.md
Outdated
|
|
||
| ## Motivation | ||
|
|
||
| The current AgentCube sandbox implementation relies on SSH (via `ssh_client.py`) for remote code execution, file transfer, and sandbox management. While SSH provides robust authentication and encryption, it introduces several challenges: |
There was a problem hiding this comment.
No sandbox management I think.
docs/design/PicoD-Design.md
Outdated
|
|
||
| When PicoD runs inside a sandbox container, the access token must be securely provided at startup. Several options are available depending on the deployment environment: | ||
|
|
||
| ##### Option 1: Kubernetes Secret Mount (Recommended for K8s) |
There was a problem hiding this comment.
Now SDK would generate private/public key and forward public key to sandbox pod. Then this use public key to authenticate SSH connections. Is this an option?
docs/design/PicoD-Design.md
Outdated
| 2. `PICOD_ACCESS_TOKEN` environment variable | ||
| 3. `PICOD_ACCESS_TOKEN_FILE` environment variable (reads from file) | ||
| 4. `/etc/picod/token` default file location | ||
| 5. Instance metadata service (if configured) |
There was a problem hiding this comment.
So we will support all four authentication methods mentioned above?
docs/design/PicoD-Design.md
Outdated
|
|
||
| ## Conclusion | ||
|
|
||
| PicoD provides a lightweight, efficient alternative to SSH for sandbox management in AgentCube. By leveraging modern HTTP/gRPC protocols and token-based authentication, it reduces resource overhead while maintaining security and functionality. The design ensures easy integration with existing AgentCube infrastructure and provides a clear migration path from the current SSH-based implementation. |
There was a problem hiding this comment.
| PicoD provides a lightweight, efficient alternative to SSH for sandbox management in AgentCube. By leveraging modern HTTP/gRPC protocols and token-based authentication, it reduces resource overhead while maintaining security and functionality. The design ensures easy integration with existing AgentCube infrastructure and provides a clear migration path from the current SSH-based implementation. | |
| PicoD provides a lightweight, efficient alternative to SSH for sandbox access in AgentCube. By leveraging modern HTTP/gRPC protocols and token-based authentication, it reduces resource overhead while maintaining security and functionality. The design ensures easy integration with existing AgentCube infrastructure and provides a clear migration path from the current SSH-based implementation. |
LaynePeng
force-pushed
the
picod-design
branch
from
8ebb78f to
7867ed0
Compare
| - **Secure**: Token-based authentication, eliminating the need for preconfigured users or SSH keys. | ||
| - **No Lifecycle Management**: Sandbox lifecycle (creation, deletion, monitoring) remains the responsibility of the AgentCube control plane. PicoD focuses solely on request handling. | ||
| - **Single-Request Processing**: Each API call (Execute, ReadFile, WriteFile) is handled independently, without shared state. | ||
| - **No Session Management**: No persistent connections or session tracking; every request is authenticated via metadata. |
| **File Access Control** | ||
|
|
||
| - Path sanitization prevents directory traversal | ||
| - Restricted to sandbox workspace only |
There was a problem hiding this comment.
What specific technique does PicoD use to restrict the workspace?
| **Upload File**: | ||
|
|
||
| - **Endpoint**: `POST /api/files` | ||
| - **Option 1: Multipart Form Data** (recommended for binary files) |
There was a problem hiding this comment.
Why is the multipart form data recommended?
|
|
||
| ``` | ||
|
|
||
| - **Option 2: JSON with Base64** (for text files or API convenience) |
There was a problem hiding this comment.
Does it handle the creation of parent directories automatically during request if not exist?
|
|
||
| #### 1. HTTP Server Layer (Go Implementation) | ||
|
|
||
| - **Framework**: Gin (lightweight HTTP web framework) |
There was a problem hiding this comment.
What were the specific factors to choose Go and the Gin over Python(Flask) or other frameworks?
| @@ -0,0 +1,652 @@ | |||
| # PicoD Design Document | |||
There was a problem hiding this comment.
As we come to an aggrement this is evald now?
| - **No Lifecycle Management**: Sandbox lifecycle (creation, deletion, monitoring) remains the responsibility of the AgentCube control plane. PicoD focuses solely on request handling. | ||
| - **Single-Request Processing**: Each API call (Execute, ReadFile, WriteFile) is handled independently, without shared state. | ||
| - **No Session Management**: No persistent connections or session tracking; every request is authenticated via metadata. | ||
| - **Ephemeral Operation**: PicoD runs only for the lifetime of the sandbox container and does not track lifecycle events. |
There was a problem hiding this comment.
| - **Ephemeral Operation**: PicoD runs only for the lifetime of the sandbox container and does not track lifecycle events. | |
| - **Ephemeral Operation**: PicoD runs only during the lifetime of the sandbox container and does not track sandbox lifecycle events. |
|
|
||
| ### Machine Learning Workflow | ||
|
|
||
| An AI agent performs a complete machine learning workflow - uploading data, installing dependencies, training a model, and downloading results: |
There was a problem hiding this comment.
The use case seems not very appropriate, taining is not our primary use case
docs/design/PicoD-Design.md
Outdated
|
|
||
| ### Core API Endpoints | ||
|
|
||
| 1. **POST /api/execute** - Execute commands |
There was a problem hiding this comment.
So does this mean you wonot provide a separate run code interface? And it work,s like we currently implement run_code calling execute_command in sdk
docs/design/PicoD-Design.md
Outdated
|
|
||
| 1. **POST /api/execute** - Execute commands | ||
| 2. **POST /api/files** - Upload files | ||
| 3. **GET /api/files/{path}** - Download files |
There was a problem hiding this comment.
should we use query parameter to specify complex file path
like api/files?file={path}
docs/design/PicoD-Design.md
Outdated
| ### Core API Endpoints | ||
|
|
||
| 1. **POST /api/execute** - Execute commands | ||
| 2. **POST /api/files** - Upload files |
There was a problem hiding this comment.
any details how to set the filepath to store and how the file content is sent
I think this should be HTTP multipart
| HTTPServer[HTTP Server<br/>Port: 9527] | ||
| AuthMiddleware[Auth Middleware] | ||
| LogMiddleware[Logging Middleware] | ||
| ErrorMiddleware[Error Handler] |
VanderChen
force-pushed
the
picod-design
branch
2 times, most recently
from
088725e to
18cdd20
Compare
Signed-off-by: VanderChen <vanderchen@outlook.com>
VanderChen
force-pushed
the
picod-design
branch
from
863d4dd to
9243f38
Compare
| 1. **POST /tools/code-interpreter/execute** - Execute commands | ||
| 2. **POST /tools/code-interpreter/files** - Upload files | ||
| 3. **GET /tools/code-interpreter/files/{path}** - Download files | ||
| 4. **GET /tools/code-interpreter/health** - Health check endpoint |
There was a problem hiding this comment.
should simplify by removing code-interpreter, maybe
/v1/execute
/v1/files
| - Request: JSON with command, timeout, env vars | ||
| - Response: JSON with stdout, stderr, exit_code |
| ```json | ||
| { | ||
| "auth_type": "token|keypair", | ||
| "token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...", |
There was a problem hiding this comment.
what is this token? If this is the default token used by picode to auth agent-gateway, we should take it in header
| { | ||
| "auth_type": "token|keypair", | ||
| "token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...", | ||
| "public_key": "-----BEGIN PUBLIC KEY-----\n...", |
There was a problem hiding this comment.
is this used by picod to auth client?
5 participants
What type of PR is this?
/kind documentation
/kind feature
What this PR does / why we need it:
This PR proposes PicoD (Pico Daemon) - a lightweight, HTTP-based service daemon that provides essential sandbox capabilities with minimal overhead while maintaining security through token-based authentication.
Which issue(s) this PR fixes:
Fix #41