-
Notifications
You must be signed in to change notification settings - Fork 28
docs: add aenvironment-deploy skill and instance deployment API docum… #65
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
base: main
Are you sure you want to change the base?
Conversation
…entation - Add aenvironment-deploy skill for deploying instances and services - Add Chinese instance deployment API documentation for AI agents - Fix markdownlint issues in all markdown files - Include Python implementation reference and parameter quick reference table Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @JacksonMei, 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 significantly enhances the AEnvironment platform by introducing a new Claude Code skill designed to simplify and automate the deployment of sandboxed environments. It provides flexible workflows for deploying both temporary agent instances and persistent application services, backed by detailed documentation and a robust Python library for managing these operations. 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. Changelog
Activity
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
|
puzhen-ryan
left a comment
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.
lgtm
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 new aenvironment-deploy skill, a valuable addition for automating deployments. A critical security vulnerability was identified where the AEnvOperations library leaks sensitive data, including passwords and secrets, to stderr in verbose logs; implementing argument masking for sensitive fields is strongly recommended. Furthermore, the aenv_operations.py script's reliance on parsing human-readable CLI output is fragile, and improvements for more reliable, machine-readable parsing are suggested. Minor issues include a documentation discrepancy and an unused variable.
| # Extract instance info from output | ||
| instance_id = "unknown" | ||
| ip_address = "unknown" | ||
|
|
||
| for line in stdout.split("\n"): | ||
| if "id" in line.lower(): | ||
| parts = line.split() | ||
| if len(parts) >= 2: | ||
| instance_id = parts[-1] | ||
| elif "ip" in line.lower() or "address" in line.lower(): | ||
| parts = line.split() | ||
| for part in parts: | ||
| if self._is_valid_ip(part): | ||
| ip_address = part | ||
|
|
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.
Extracting instance_id and ip_address by parsing the command's standard output is highly unreliable. This approach is sensitive to any changes in the output text, including wording, capitalization, or formatting. A much more robust solution would be for the aenv instance create command to return structured data, like JSON, which can be parsed reliably. This would prevent potential failures where the script might not be able to retrieve critical deployment information.
| # Extract service info from log output and table | ||
| service_id = "unknown" | ||
| access_url = "unknown" | ||
|
|
||
| for line in stdout.split("\n"): | ||
| # Look for service ID in log line or table | ||
| if "service created:" in line.lower(): | ||
| parts = line.split(":") | ||
| if len(parts) >= 2: | ||
| service_id = parts[-1].strip().rstrip('[0m') | ||
| elif "│ service id" in line.lower(): | ||
| parts = [p.strip() for p in line.split("│") if p.strip()] | ||
| if len(parts) >= 2: | ||
| service_id = parts[1] | ||
| elif "│ service url" in line.lower(): | ||
| parts = [p.strip() for p in line.split("│") if p.strip()] | ||
| if len(parts) >= 2: | ||
| access_url = parts[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.
Similar to create_instance, this function parses human-readable output to get the service_id and access_url. This is very fragile and can easily break if the CLI output changes. The aenv service create command should ideally provide a machine-readable output format (e.g., JSON) to allow for robust parsing of the results.
| for attempt in range(retry + 1): | ||
| try: | ||
| if self.verbose: | ||
| print(f"Running: {' '.join(cmd)}", file=sys.stderr) |
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 _run_command method prints the full command being executed to sys.stderr when the verbose flag is enabled. This command can contain sensitive information such as registry passwords (passed via configure_cli) or environment variables (passed via create_instance, create_service, or update_service). If a user runs any of the deployment scripts with the --verbose flag, their credentials and secrets will be logged to the terminal's error stream, which may be captured in CI/CD logs or shared terminal sessions.
To remediate this, implement a masking mechanism to redact sensitive values before logging. For example, you can identify arguments that follow sensitive flags like -e or are part of configuration commands and replace their values with placeholders.
| python -c "from scripts.aenv_operations import AEnvOperations; \ | ||
| ops = AEnvOperations(); \ | ||
| ops.configure_cli('owner', 'api-url', 'hub-url'); \ | ||
| print(ops.list_instances())" |
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 placeholders owner, api-url, and hub-url in the example command are a bit vague. For better clarity and consistency with other parts of the documentation, consider using more descriptive placeholders like <owner-name>, <api-service-url>, and <envhub-url>. This will make it easier for users to understand what values they need to provide. A similar issue exists on lines 200-203.
| python -c "from scripts.aenv_operations import AEnvOperations; \ | |
| ops = AEnvOperations(); \ | |
| ops.configure_cli('owner', 'api-url', 'hub-url'); \ | |
| print(ops.list_instances())" | |
| python -c "from scripts.aenv_operations import AEnvOperations; \ | |
| ops = AEnvOperations(); \ | |
| ops.configure_cli('<owner-name>', '<api-service-url>', '<hub-url>'); \ | |
| print(ops.list_instances())" |
| # Parse environment names from rich table output | ||
| envs = [] | ||
| in_table = False | ||
| for line in stdout.split("\n"): | ||
| line = line.strip() | ||
| # Skip empty lines, headers, and table decorations | ||
| if not line or line.startswith("Available") or line.startswith("┏") or \ | ||
| line.startswith("┃") or line.startswith("┡") or line.startswith("└"): | ||
| continue | ||
| # Data rows start with │ | ||
| if line.startswith("│"): | ||
| parts = [p.strip() for p in line.split("│") if p.strip()] | ||
| if len(parts) >= 2: # name and version | ||
| env_name = parts[0] | ||
| version = parts[1] | ||
| envs.append(f"{env_name}@{version}") | ||
|
|
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.
Parsing the human-readable table output from the aenv list command is very brittle. Any change in the CLI's output formatting, including changes to the box-drawing characters or column spacing, will break this parsing logic. To make this more robust, the aenv CLI should be updated to provide a machine-readable output format, such as JSON (e.g., aenv list --output json). This would allow for reliable parsing and eliminate the need for this complex and fragile string manipulation.
|
|
||
| # Parse environment names from rich table output | ||
| envs = [] | ||
| in_table = False |
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.
| if "service created:" in line.lower(): | ||
| parts = line.split(":") | ||
| if len(parts) >= 2: | ||
| service_id = parts[-1].strip().rstrip('[0m') |
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 rstrip('[0m') to remove ANSI escape codes is not a robust solution, as it only handles one specific escape sequence at the end of the string. Other color codes or text formatting sequences will not be removed. A more reliable approach is to use a regular expression to strip all ANSI escape codes from the string. For example:
import re
clean_text = re.sub(r'\x1B\[[0-?]*[ -/]*[@-~]', '', text_with_ansi)This would require importing the re module at the top of the file.
…entation