Add GitHub Copilot and ai agents support - #16
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 095a07b9ac
ℹ️ 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".
| echo " ✓ $hook is executable" | ||
| else | ||
| echo " ✗ $hook is missing or not executable" | ||
| ((ERRORS++)) |
There was a problem hiding this comment.
Avoid post-increment counters under errexit
Because the script enables set -e, each ((ERRORS++))/((WARNINGS++)) can terminate execution when the prior value is 0 (the arithmetic command exits non-zero in that case). This means the verifier exits on the first missing hook or warning instead of completing checks and printing the summary/fix guidance; for example, running the script in this repo stops immediately after the first failed hook check.
Useful? React with 👍 / 👎.
| # Get branch name from worktree | ||
| cd "$WORKTREE_PATH" | ||
| BRANCH="$(git branch --show-current)" | ||
| cd - >/dev/null |
There was a problem hiding this comment.
Keep cleanup git commands in the owning repository
The script captures the branch from the worktree and then cd - back to the caller’s previous directory before running git worktree remove and branch deletion commands. If the caller invokes this script from outside the main repository (using an absolute worktree path), those later git commands fail with not a git repository, so valid cleanup requests cannot complete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds GitHub Copilot “agent” documentation and a git-worktree operations skillset/scripts to support multi-agent workflows in this repo, along with contributor-facing project guidance.
Changes:
- Added contributor and Copilot workspace instruction docs (
CONTRIBUTING.md,.github/copilot-instructions.md). - Introduced three agent definition files (
ArchitectReviewer,PackagePrepper,TestWrangler). - Added git worktree helper scripts and a detailed worktree operations skill doc under
.agents/skills/git-worktree-ops/.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
CONTRIBUTING.md |
New contribution guide (build/test/changelog/checklists). |
.github/copilot-instructions.md |
New Copilot workspace guidance and repo conventions summary. |
.github/agents/architectreviewer.agent.md |
Agent definition for architecture / multi-TFM validation. |
.github/agents/packageprepper.agent.md |
Agent definition for release/versioning/changelog workflow. |
.github/agents/testwrangler.agent.md |
Agent definition for MSTest creation/execution/coverage workflow. |
.agents/skills/git-worktree-ops/verify-guardrails.sh |
Script to validate per-worktree guardrail setup. |
.agents/skills/git-worktree-ops/quick-worktree.sh |
Script to create a new worktree + configure guardrails. |
.agents/skills/git-worktree-ops/cleanup-worktree.sh |
Script to remove worktrees and optionally delete branches/remotes. |
.agents/skills/git-worktree-ops/SKILL.md |
Large reference doc for worktree ops + guardrails workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [[ "$CURRENT_BRANCH" != "$EXPECTED_BRANCH" ]]; then | ||
| echo " ⚠️ WARNING: Current branch ($CURRENT_BRANCH) != expected ($EXPECTED_BRANCH)" | ||
| ((WARNINGS++)) | ||
| fi |
There was a problem hiding this comment.
Same issue as the ERRORS counter: with set -e, ((WARNINGS++)) may terminate the script unexpectedly when WARNINGS is 0. Prefer an increment form that returns success (e.g., WARNINGS+=1) for all WARNINGS increments in this script.
| Before starting agent work in a worktree: | ||
| - [ ] Worktree created with correct branch name | ||
| - [ ] Branch follows naming convention `feat/<agent>-<task>` | ||
| - [ ] Metadata files written to `.git/agent-expected-branch` and `.git/agent-expected-worktree` |
There was a problem hiding this comment.
This checklist item points contributors at .git/agent-expected-*, but in a worktree the effective git dir is often something like .git/worktrees/<name>. To avoid confusion, reference writing these files via $(git rev-parse --git-dir)/agent-expected-* (as shown earlier in this doc and scripts) instead of hard-coding .git/....
| - [ ] Metadata files written to `.git/agent-expected-branch` and `.git/agent-expected-worktree` | |
| - [ ] Metadata files written to `$(git rev-parse --git-dir)/agent-expected-branch` and `$(git rev-parse --git-dir)/agent-expected-worktree` |
| else | ||
| echo " ✗ $hook is missing or not executable" | ||
| ((ERRORS++)) | ||
| fi |
There was a problem hiding this comment.
Because the script runs with set -e, using ((ERRORS++)) can cause an early exit the first time it runs (the arithmetic command returns status 1 when the pre-increment value is 0). Use an increment form that returns success (e.g., ERRORS+=1) or otherwise avoid errexit for these counters; make the same change for the other counter increments in this file as well.
| ;; | ||
| esac | ||
| done | ||
|
|
There was a problem hiding this comment.
The usage text says --remote requires --delete-branch, but the script currently accepts --remote on its own and then silently does nothing. Add an explicit validation after option parsing to error out (or implicitly enable --delete-branch) when --remote is provided without --delete-branch.
| # Validate option combinations | |
| if [[ $DELETE_REMOTE -eq 1 && $DELETE_BRANCH -ne 1 ]]; then | |
| echo "Error: --remote requires --delete-branch" >&2 | |
| usage | |
| exit 1 | |
| fi |
|
|
||
| ### Build Script | ||
|
|
||
| Use `build.ps1` for full CI pipeline (restore tools, versioning, build, test, pack). |
There was a problem hiding this comment.
This states build.ps1 runs tests as part of the full pipeline, but build.ps1 currently has the dotnet test step commented out. Either update the script to actually run tests, or adjust this documentation so contributors don’t assume tests were executed by build.ps1.
| Use `build.ps1` for full CI pipeline (restore tools, versioning, build, test, pack). | |
| Use `build.ps1` for CI pipeline steps (restore tools, versioning, build, pack). Run tests separately with `dotnet test` as needed. |
| echo " printf '%s\\n' \"$CURRENT_BRANCH\" > \"\$GIT_DIR/agent-expected-branch\"" | ||
| echo " printf '%s\\n' \"$CURRENT_PATH\" > \"\$GIT_DIR/agent-expected-worktree\"" |
There was a problem hiding this comment.
These echo lines construct shell commands that interpolate CURRENT_BRANCH and CURRENT_PATH directly, so if a branch name contains shell metacharacters (for example $(), backticks, semicolons, or quotes) and a user copy-pastes the suggested "To fix, run" commands, the shell will execute attacker-controlled payloads. An attacker who can create a branch with a malicious name (e.g., via a compromised or untrusted remote) could achieve arbitrary command execution on a developer machine when this script is run and its output is followed. To fix, avoid echoing fully-resolved commands that embed untrusted values (use placeholders or robust escaping), or apply these configuration changes directly in the script instead of asking users to run dynamically-generated shell commands.
| echo " printf '%s\\n' \"$CURRENT_BRANCH\" > \"\$GIT_DIR/agent-expected-branch\"" | |
| echo " printf '%s\\n' \"$CURRENT_PATH\" > \"\$GIT_DIR/agent-expected-worktree\"" | |
| echo " printf '%s\\n' \"<CURRENT_BRANCH>\" > \"\$GIT_DIR/agent-expected-branch\"" | |
| echo " printf '%s\\n' \"<CURRENT_PATH>\" > \"\$GIT_DIR/agent-expected-worktree\"" |
- Introduced `cleanup-worktree.sh` for removing completed or abandoned Git worktrees, with options to delete branches and remote branches. - Added `quick-worktree.sh` for setting up new worktrees with automatic guardrails configuration. - Created `verify-guardrails.sh` to ensure proper guardrail configurations in current worktrees. - Added `ArchitectReviewer` agent for validating architectural patterns and multi-framework compatibility. - Introduced `PackagePrepper` agent for managing the release pipeline, including versioning and changelog management. - Created `TestWrangler` agent for test creation, execution, and coverage analysis in MSTest projects. - Added comprehensive instructions in `copilot-instructions.md` for project setup and development guidelines. - Established a `CONTRIBUTING.md` file to guide contributors on project structure, code conventions, and testing procedures.
095a07b to
396a379
Compare
cleanup-worktree.shfor removing completed or abandoned Git worktrees, with options to delete branches and remote branches.quick-worktree.shfor setting up new worktrees with automatic guardrails configuration.verify-guardrails.shto ensure proper guardrail configurations in current worktrees.ArchitectRevieweragent for validating architectural patterns and multi-framework compatibility.PackagePrepperagent for managing the release pipeline, including versioning and changelog management.TestWrangleragent for test creation, execution, and coverage analysis in MSTest projects.copilot-instructions.mdfor project setup and development guidelines.CONTRIBUTING.mdfile to guide contributors on project structure, code conventions, and testing procedures.