Import agent-config baseline from brianlovin - #1
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (49)
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive agent-config management system with installation and synchronization utilities, documentation for testing and workflows, multiple reusable skills for various development tasks, and an automated test suite. The system enables management of Claude/Codex agent configurations, skills, and settings across machines with backup and version control integration. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as install.sh
participant Env as Environment
participant FS as Filesystem
participant BK as Backup System
participant Git as Git/Repo
User->>CLI: ./install.sh [--dry-run] [--force]
CLI->>Env: Parse options, init HOME paths
CLI->>FS: Check for existing config
alt Conflicts detected
CLI->>User: Prompt for resolution (overwrite/keep/diff/quit)
User->>CLI: Choose action
CLI->>BK: Create timestamped backup
BK->>FS: Store original files
BK->>FS: Write manifest.json
else No conflicts
CLI->>BK: Create preemptive backup
end
CLI->>FS: Create symlinks for settings.json
CLI->>FS: Create symlinks for statusline.sh
CLI->>FS: Link skills, agents, rules directories
CLI->>Git: Validate repo items (SKILL.md check)
alt Dry-run mode
CLI->>User: Print intended actions
else Normal run
CLI->>FS: Apply all symlinks
CLI->>BK: Write operation manifest
end
CLI->>User: Report success/backup location
Note over User,Git: User can now sync changes or undo via sync.sh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The changes span heterogeneous file types (shell scripts, Python, documentation, configuration, tests) with high logic density in install.sh (~369 lines with conflict detection, backup, and symlink logic) and sync.sh (~710 lines with discovery, validation, git integration). While individual documentation files are straightforward, the orchestration logic across multiple scripts, backup mechanisms, and manifest management requires careful review of state transitions and error handling paths. Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, 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 integrates a foundational agent configuration system, enabling efficient management and synchronization of AI agent settings, skills, and rules. It introduces automated installation and synchronization scripts, a comprehensive testing framework, and a diverse set of pre-configured agents and skills. The changes aim to streamline AI-driven development by providing structured configuration, robust testing, and clear operational workflows, significantly enhancing the project's overall capability and maintainability. 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
Ignored Files
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
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa5443ef46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,198 @@ | |||
| --- | |||
There was a problem hiding this comment.
sync.sh validate only accepts skills that contain SKILL.md, and this directory is the only one added with a lowercase skill.md; on a clean checkout ./sync.sh validate fails with “Missing SKILL.md,” which also breaks the CI workflow step in .github/workflows/test.yml that runs validation. This makes the repository fail its own required validation path until the filename matches the expected convention.
Useful? React with 👍 / 👎.
| for item in "$backup_root"/*; do | ||
| [ -e "$item" ] || continue | ||
| local item_name=$(basename "$item") | ||
| local root_name=$(basename "$backup_root") | ||
| local dest="$HOME/$root_name/$item_name" |
There was a problem hiding this comment.
undo_last only iterates direct children of $backup_path/.claude and $backup_path/.codex, but symlink backups are stored deeper (for example .claude/skills/<name>.symlink), so those entries are never handled by the symlink branch and end up copied as plain *.symlink files instead of real links. In practice, undoing operations like remove skill does not restore the original symlinks and leaves broken local state.
Useful? React with 👍 / 👎.
| elif [ -d "$item" ]; then | ||
| rm -rf "$dest" | ||
| cp -r "$item" "$dest" |
There was a problem hiding this comment.
When a backed-up item is a directory, undo does rm -rf "$dest" and then copies the backup directory wholesale, which wipes unrelated local content under that directory that was never part of the operation. For example, undoing a backup that only contains one skill under .claude/skills removes other local skills before restore, causing unintended data loss.
Useful? React with 👍 / 👎.
![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 baseline for agent configuration, including installation and synchronization scripts, a full test suite using Bats, and a wide array of skills and documentation. The overall quality of the scripts and tests is high, demonstrating robust and thoughtful implementation. My feedback focuses on minor improvements in documentation clarity, fixing typos, and enhancing the robustness of some Python helper scripts by suggesting the use of argparse for more flexible command-line argument handling.
| ```bash | ||
| agent-browser open example.com --headed # Show browser window | ||
| agent-browser console # View console messages | ||
| agent-browser errors # View page errors | ||
| agent-browser record start ./debug.webm # Record from current page | ||
| agent-browser record stop # Save recording | ||
| agent-browser open example.com --headed # Show browser window | ||
| agent-browser --cdp 9222 snapshot # Connect via CDP | ||
| agent-browser console # View console messages | ||
| agent-browser console --clear # Clear console | ||
| agent-browser errors # View page errors | ||
| agent-browser errors --clear # Clear errors | ||
| agent-browser highlight @e1 # Highlight element | ||
| agent-browser trace start # Start recording trace | ||
| agent-browser trace stop trace.zip # Stop and save trace | ||
| ``` No newline at end of file |
There was a problem hiding this comment.
The Debugging section contains several duplicated commands, which appears to be a copy-paste error. For clarity and to avoid confusion, I recommend removing the redundant entries.
Specifically, the following commands are duplicated:
agent-browser open example.com --headed(lines 238 and 243)agent-browser console(lines 239 and 245)agent-browser errors(lines 240 and 247)
| ```bash | |
| agent-browser open example.com --headed # Show browser window | |
| agent-browser console # View console messages | |
| agent-browser errors # View page errors | |
| agent-browser record start ./debug.webm # Record from current page | |
| agent-browser record stop # Save recording | |
| agent-browser open example.com --headed # Show browser window | |
| agent-browser --cdp 9222 snapshot # Connect via CDP | |
| agent-browser console # View console messages | |
| agent-browser console --clear # Clear console | |
| agent-browser errors # View page errors | |
| agent-browser errors --clear # Clear errors | |
| agent-browser highlight @e1 # Highlight element | |
| agent-browser trace start # Start recording trace | |
| agent-browser trace stop trace.zip # Stop and save trace | |
| ``` | |
| agent-browser open example.com --headed # Show browser window | |
| agent-browser --cdp 9222 snapshot # Connect via CDP | |
| agent-browser console # View console messages | |
| agent-browser console --clear # Clear console | |
| agent-browser errors # View page errors | |
| agent-browser errors --clear # Clear errors | |
| agent-browser highlight @e1 # Highlight element | |
| agent-browser record start ./debug.webm # Record from current page | |
| agent-browser record stop # Save recording | |
| agent-browser trace start # Start recording trace | |
| agent-browser trace stop trace.zip # Stop and save trace |
| "build:main": "esbuild src/main/index.ts --bundle --platform=node --format=esm --outfile=dist/main/main/index.js --external:electron --banner:js=\"import { createRequire } from 'module'; var require = createRequire(import.meta.url);\"", | ||
| "build:preload": "esbuild src/preload/index.ts --bundle --platform=node --format=cjs --outfile=dist/preload/preload/index.js --external:electron" |
There was a problem hiding this comment.
The outfile paths in the build:main and build:preload scripts appear to have duplicated directory names (dist/main/main/index.js and dist/preload/preload/index.js). This seems inconsistent with the configuration in project-setup.md, which points to dist/main/index.js and dist/preload/index.js respectively. This could cause issues if a user copies these commands directly.
| "build:main": "esbuild src/main/index.ts --bundle --platform=node --format=esm --outfile=dist/main/main/index.js --external:electron --banner:js=\"import { createRequire } from 'module'; var require = createRequire(import.meta.url);\"", | |
| "build:preload": "esbuild src/preload/index.ts --bundle --platform=node --format=cjs --outfile=dist/preload/preload/index.js --external:electron" | |
| "build:main": "esbuild src/main/index.ts --bundle --platform=node --format=esm --outfile=dist/main/index.js --external:electron --banner:js=\"import { createRequire } from 'module'; var require = createRequire(import.meta.url);\"", | |
| "build:preload": "esbuild src/preload/index.ts --bundle --platform=node --format=cjs --outfile=dist/preload/index.js --external:electron" |
| - CHANGELOG.md | ||
| - etc. | ||
|
|
||
| The skill should only contain the information needed for an AI agent to do the job at hand. It should not contain auxilary context about the process that went into creating it, setup and testing procedures, user-facing documentation, etc. Creating additional documentation files just adds clutter and confusion. |
There was a problem hiding this comment.
There is a typo in the word "auxiliary".
| The skill should only contain the information needed for an AI agent to do the job at hand. It should not contain auxilary context about the process that went into creating it, setup and testing procedures, user-facing documentation, etc. Creating additional documentation files just adds clutter and confusion. | |
| The skill should only contain the information needed for an AI agent to do the job at hand. It should not contain auxiliary context about the process that went into creating it, setup and testing procedures, user-facing documentation, etc. Creating additional documentation files just adds clutter and confusion. |
| if len(sys.argv) < 4 or sys.argv[2] != '--path': | ||
| print("Usage: init_skill.py <skill-name> --path <path>") | ||
| print("\nSkill name requirements:") | ||
| print(" - Hyphen-case identifier (e.g., 'data-analyzer')") | ||
| print(" - Lowercase letters, digits, and hyphens only") | ||
| print(" - Max 40 characters") | ||
| print(" - Must match directory name exactly") | ||
| print("\nExamples:") | ||
| print(" init_skill.py my-new-skill --path skills/public") | ||
| print(" init_skill.py my-api-helper --path skills/private") | ||
| print(" init_skill.py custom-skill --path /custom/location") | ||
| sys.exit(1) |
There was a problem hiding this comment.
The command-line argument parsing is brittle as it relies on a fixed argument order. Using Python's argparse module would make the script more robust and user-friendly. It would allow for flexible argument ordering (e.g., init_skill.py --path <path> <skill-name>) and automatically provide a helpful --help message.
| if len(sys.argv) < 2: | ||
| print("Usage: python utils/package_skill.py <path/to/skill-folder> [output-directory]") | ||
| print("\nExample:") | ||
| print(" python utils/package_skill.py skills/public/my-skill") | ||
| print(" python utils/package_skill.py skills/public/my-skill ./dist") | ||
| sys.exit(1) |
There was a problem hiding this comment.
The argument parsing in this script is dependent on a specific order and doesn't use named arguments for clarity. I recommend using the argparse module to make it more robust, especially for handling the optional output-directory. This would also provide a standard --help message for users.
| if len(sys.argv) != 2: | ||
| print("Usage: python quick_validate.py <skill_directory>") | ||
| sys.exit(1) |
There was a problem hiding this comment.
The argument parsing is very basic. While it works for a single argument, using the argparse module is a best practice that would make the script more maintainable and provide a standard --help output for better usability.
1 participant
This PR imports the agent-config baseline into this repo from brianlovin/agent-config. It adds install and sync scripts, testing docs/workflow, and the full Bats test suite. It also includes agents, skills, and supporting settings and statusline files for symlink-based config management. No existing files were modified beyond adding this new configuration set.
Summary by CodeRabbit
New Features
Documentation
Tests