-
Notifications
You must be signed in to change notification settings - Fork 442
feat: add simple webui #827
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
Conversation
Summary of ChangesHello @vinci-grape, 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 adds a comprehensive web UI to the MS-Agent framework, significantly enhancing its usability and accessibility. The new UI provides a user-friendly way to manage projects, configure agents, monitor their execution, and view their output in real-time. This addition makes MS-Agent more approachable for users who prefer a graphical interface over command-line interactions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 comprehensive Web UI for MS-Agent, including a FastAPI backend and a React frontend. The overall structure is well-designed, with clear separation of concerns between frontend and backend components. The backend correctly uses asynchronous programming for handling long-running agent tasks and WebSockets for real-time communication. The frontend provides a polished and intuitive user experience. I've identified a few critical security and correctness issues that should be addressed, primarily related to insecure handling of secrets, potential path traversal, overly permissive CORS settings, and a missing dependency. I've also included some medium-severity suggestions to improve code quality and maintainability.
| cmd.extend(['--openai_api_key', llm_config['api_key']]) | ||
|
|
||
| elif project_type == 'script': | ||
| # Run the script directly | ||
| cmd = [python, self.project['config_file']] | ||
| else: | ||
| cmd = [python, '-m', 'ms_agent', 'run', '--config', project_path] | ||
|
|
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.
Passing API keys as command-line arguments is a security risk, as they can be exposed to other users on the system through process lists (e.g., ps command). The application already correctly sets these keys as environment variables in _build_env. You should rely solely on environment variables for passing secrets and remove the logic that adds them as command-line arguments.
|
|
||
| return result | ||
|
|
||
| tree = build_tree(output_dir) | ||
| return {'tree': tree, 'output_dir': output_dir} | ||
|
|
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 security check to prevent path traversal is a good step, but it can be bypassed if the allowed directories contain symbolic links pointing outside. To mitigate this, you should resolve the real path of both the requested file and the allowed directories using os.path.realpath() before performing the check. This ensures you are comparing canonical, absolute paths without any symlinks.
| return result | |
| tree = build_tree(output_dir) | |
| return {'tree': tree, 'output_dir': output_dir} | |
| # Security check: ensure file is within allowed directories | |
| allowed_dirs_real = [os.path.realpath(output_dir), os.path.realpath(projects_dir)] | |
| real_path = os.path.realpath(full_path) | |
| is_allowed = any(real_path.startswith(d) for d in allowed_dirs_real) | |
| if not is_allowed: | |
| raise HTTPException(status_code=403, detail="Access denied: file outside allowed directories") |
| app.add_middleware( | ||
| CORSMiddleware, | ||
| allow_origins=['*'], | ||
| allow_credentials=True, | ||
| allow_methods=['*'], | ||
| allow_headers=['*'], | ||
| ) |
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 CORS configuration allow_origins=["*"] is too permissive for a production environment, as it allows any website to make requests to your backend. This can lead to security vulnerabilities. You should restrict the allowed origins to the specific URL of your frontend application.
| app.add_middleware( | |
| CORSMiddleware, | |
| allow_origins=['*'], | |
| allow_credentials=True, | |
| allow_methods=['*'], | |
| allow_headers=['*'], | |
| ) | |
| app.add_middleware( | |
| CORSMiddleware, | |
| # In production, you should restrict this to your frontend's domain. | |
| # For development, you can use a list of allowed origins. | |
| allow_origins=["http://localhost:5173", "http://127.0.0.1:5173"], # Example for dev | |
| allow_credentials=True, | |
| allow_methods=["*"], | |
| allow_headers=["*"], | |
| ) |
| def get_project_config(self, project_id: str) -> Optional[Dict[str, Any]]: | ||
| """Get the configuration for a project""" | ||
| project = self.get_project(project_id) | ||
| if not project: |
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.
|
|
||
| elif project_type == 'script': | ||
| # script类型:直接运行Python脚本 | ||
| cmd = [python, self.project['config_file']] # run.py |
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 code snippet for _build_command uses a python variable that is not defined within the snippet. While it's defined in the actual source code, for documentation clarity, it would be better to use a literal string like 'python' to make the example self-contained and easier to understand.
| cmd = [python, self.project['config_file']] # run.py | |
| cmd = ['python', self.project['config_file']] # run.py |
| const [input, setInput] = useState(''); | ||
| const [outputFilesOpen, setOutputFilesOpen] = useState(false); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const [outputTree, setOutputTree] = useState<any>({folders: {}, files: []}); |
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.
Using any for the outputTree state weakens TypeScript's type safety. You should define a specific interface for the file tree structure to improve maintainability and prevent potential runtime errors.
For example:
interface FileNode {
name: string;
path: string;
size: number;
modified: number;
}
interface FolderNode {
folders: Record<string, FolderNode>;
files: FileNode[];
}
const [outputTree, setOutputTree] = useState<FolderNode>({folders: {}, files: []});| if [ ! -d "$SCRIPT_DIR/frontend/node_modules" ]; then | ||
| echo -e "${YELLOW}Installing frontend dependencies...${NC}" | ||
| cd "$SCRIPT_DIR/frontend" | ||
| npm install |
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.
For faster and more deterministic dependency installation, especially in CI/CD environments, it's recommended to use npm ci instead of npm install. npm ci performs a clean install based on the package-lock.json file, which ensures that the exact same dependency versions are used every time.
| npm install | |
| npm ci |
Change Summary
MS-Agent WebUI

Related issue number
Checklist
pre-commit installandpre-commit run --all-filesbefore git commit, and passed lint check.