Skip to content

Commit d089d4a

Browse files
karthiknadigkarthiknadig
committed
chore: add pr-shepherd agent and rust-locator-patterns skill
1 parent f94565d commit d089d4a

2 files changed

Lines changed: 278 additions & 0 deletions

File tree

‎.github/agents/pr-shepherd.agent.md‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
---
2+
name: "pr-shepherd"
3+
description: "Autonomous PR review lifecycle agent. Pushes code, requests Copilot review, polls for comments, fixes issues, runs pre-commit checks, resolves threads, and re-requests review — looping until approved or timeout. Eliminates manual 'more comments' / 'check for comments' prompts."
4+
tools:
5+
- "execute/runInTerminal"
6+
- "execute/getTerminalOutput"
7+
- "execute/awaitTerminal"
8+
- "execute/killTerminal"
9+
- "execute/testFailure"
10+
- "read/readFile"
11+
- "read/problems"
12+
- "read/terminalLastCommand"
13+
- "read/terminalSelection"
14+
- "edit/editFiles"
15+
- "edit/createFile"
16+
- "search"
17+
- "agent"
18+
- "github/add_comment_to_pending_review"
19+
- "github/add_issue_comment"
20+
- "github/create_pull_request"
21+
- "github/get_label"
22+
- "github/issue_read"
23+
- "github/list_branches"
24+
- "github/list_commits"
25+
- "github/list_pull_requests"
26+
- "github/merge_pull_request"
27+
- "github/pull_request_read"
28+
- "github/pull_request_review_write"
29+
- "github/request_copilot_review"
30+
- "github/search_issues"
31+
- "github/search_pull_requests"
32+
- "github/update_pull_request"
33+
- "github/update_pull_request_branch"
34+
- "todo"
35+
user-invocable: false
36+
---
37+
38+
# PR Shepherd
39+
40+
Autonomous agent that manages the PR review lifecycle end-to-end. Replaces manual "more comments" / "check for comments" / "address comments" polling.
41+
42+
## Prime Directive
43+
44+
**Drive a PR from "pushed" to "approved" with zero user intervention.** Only yield when the PR is approved, merged, or you've exhausted your retry budget.
45+
46+
---
47+
48+
## Workflow
49+
50+
### Phase 1: Setup
51+
52+
1. Identify the current PR:
53+
- If a PR URL is provided, use it
54+
- Otherwise, detect from the current branch (`git branch --show-current`) and find the matching open PR
55+
- If no PR exists, create one using `github/create_pull_request`
56+
57+
2. Ensure latest code is pushed:
58+
```
59+
git push origin <current-branch>
60+
```
61+
62+
### Phase 2: Request Review
63+
64+
1. Request Copilot review using `github/request_copilot_review`
65+
2. Wait 2 minutes for the initial review
66+
67+
### Phase 3: Poll & Process (Loop)
68+
69+
**Repeat up to 5 cycles:**
70+
71+
1. **Check for review comments:**
72+
73+
```
74+
github/pull_request_read (method: get_review_comments)
75+
```
76+
77+
2. **If no actionable comments:**
78+
- Check if PR is approved → Phase 5 (done)
79+
- If review is still pending, wait 30 seconds and re-check
80+
- After 3 consecutive empty checks, consider review complete
81+
82+
3. **If comments exist:**
83+
a. Read each unresolved, non-outdated comment
84+
b. Understand the requested change
85+
c. Make the code fix in the relevant file
86+
d. Run pre-commit checks:
87+
```bash
88+
cargo fmt --all
89+
cargo clippy --all -- -D warnings
90+
```
91+
If clippy fails, fix the errors and re-run.
92+
e. Invoke the **Reviewer** agent as a sub-agent to validate fixes
93+
f. If Reviewer finds critical issues, fix them before proceeding
94+
g. Commit the fixes: `fix: address review feedback`
95+
h. Push the changes
96+
i. Resolve addressed review threads using `gh` CLI:
97+
```bash
98+
# Get thread IDs
99+
gh api graphql -f query='{
100+
repository(owner: "OWNER", name: "REPO") {
101+
pullRequest(number: N) {
102+
reviewThreads(first: 50) {
103+
nodes { id isResolved }
104+
}
105+
}
106+
}
107+
}'
108+
109+
# Resolve each addressed thread
110+
gh api graphql -f query='mutation {
111+
resolveReviewThread(input: {threadId: "THREAD_ID"}) {
112+
thread { isResolved }
113+
}
114+
}'
115+
```
116+
j. Re-request Copilot review
117+
k. Wait 2 minutes, then loop back to step 1
118+
119+
### Phase 4: Timeout Handling
120+
121+
If after 5 full cycles there are still unresolved comments:
122+
123+
- Report remaining unresolved items to the user
124+
- Summarize what was addressed and what remains
125+
- Yield control
126+
127+
### Phase 5: Completion
128+
129+
When the PR has no actionable comments or is approved:
130+
131+
- Report "PR review complete — ready to merge" (or "PR approved")
132+
- Do NOT auto-merge unless explicitly instructed
133+
134+
---
135+
136+
## Rules
137+
138+
- **Never skip pre-commit checks** — every push must have clean `cargo fmt` + `cargo clippy`
139+
- **Never use `#[allow(...)]`** to suppress clippy warnings without user approval
140+
- **Always invoke the Reviewer agent** before pushing fixes — this catches issues before Copilot review does
141+
- **Resolve threads** after fixing — don't leave addressed comments as unresolved
142+
- **Don't argue with reviewers** — if a comment requests a change, make it. If you genuinely disagree, flag it for the user rather than ignoring it
143+
- **Extract repo owner/name** from the git remote URL or the PR context — don't hardcode
144+
145+
## Determining Repo Owner and Name
146+
147+
Parse from git remote:
148+
149+
```bash
150+
git remote get-url origin
151+
```
152+
153+
Extract owner and repo name from the URL (handles both HTTPS and SSH formats).
154+
155+
## Status Reporting
156+
157+
After each cycle, briefly report:
158+
159+
- Number of comments found vs addressed
160+
- Any comments you couldn't address (and why)
161+
- Current PR status (changes requested / pending / approved)

‎.github/skills/rust-locator-patterns/SKILL.md‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
---
2+
name: "rust-locator-patterns"
3+
description: "PET (Python Environment Tools) Rust coding conventions: locator ordering, thread safety, platform gating, JSONRPC rules, version detection, and path/symlink handling. Reference when writing or reviewing Rust code in this repo to avoid convention violations that cause review churn."
4+
---
5+
6+
# PET Rust Locator Patterns & Conventions
7+
8+
Domain knowledge for writing correct Rust code in the Python Environment Tools (PET) repository. Following these patterns prevents the most common review feedback and CI failures.
9+
10+
## Locator Ordering (CRITICAL)
11+
12+
The order of locators in `create_locators()` in `crates/pet/src/locators.rs` determines identification priority. **More specific locators MUST come before generic ones.**
13+
14+
### Priority Chain (required order):
15+
1. **Windows-specific:** Windows Store → Windows Registry → WinPython
16+
2. **Managed environments:** PyEnv → Pixi → Conda
17+
3. **Virtual envs (specific → generic):** Poetry → PipEnv → VirtualEnvWrapper → Venv → VirtualEnv
18+
4. **macOS-specific:** Homebrew → MacXCode → MacCmdLineTools → MacPythonOrg
19+
5. **Linux fallback:** LinuxGlobalPython (MUST BE LAST)
20+
21+
**Why it matters:** If `Venv` runs before `Poetry`, a Poetry environment (which IS a venv) gets misidentified as a plain venv. The first locator whose `try_from()` returns `Some` wins.
22+
23+
## Environment Identification (`try_from`)
24+
25+
Each `try_from()` must be **precise enough to avoid false positives**:
26+
27+
- **Poetry:** Match `{project-name}-{8-char-hash}-py{major}.{minor}` naming pattern AND verify location is in poetry cache dir
28+
- **Pipenv:** Look for `.project` file in centralized dir with hash-based naming
29+
- **Conda:** Require `conda-meta/` directory with history file
30+
- **Pixi:** Require `.pixi/envs/` in path structure
31+
- **venv:** Check for `pyvenv.cfg` file (but many env types have this — venv is a fallback)
32+
- **VirtualEnv:** Activation scripts WITHOUT `pyvenv.cfg` (most generic, last resort)
33+
34+
**Red flag:** A `try_from()` that only checks `pyvenv.cfg` existence — that matches ALL venvs, not just one type.
35+
36+
## JSONRPC Protocol Rules
37+
38+
The server communicates via stdin/stdout. **Any stdout pollution breaks the protocol.**
39+
40+
- **NEVER use `println!`** — it writes to stdout and corrupts JSONRPC messages
41+
- Use `log` crate macros: `info!()`, `trace!()`, `warn!()`, `error!()`
42+
- Log with elapsed time for diagnostic handlers (established pattern)
43+
- All tracing/logging writes to stderr via subscriber configuration
44+
45+
## Thread Safety Patterns
46+
47+
PET uses heavy concurrency for parallel discovery:
48+
49+
- Shared state requires `Arc<Mutex<T>>` or `Arc<RwLock<T>>`
50+
- Keep lock scopes minimal — clone data out, drop lock, then process
51+
- Use `.expect("context")` on mutex locks, not bare `.unwrap()`
52+
- No nested locks (deadlock risk)
53+
- Consider `thread::scope` for structured concurrency
54+
55+
## Platform-Specific Code
56+
57+
Use `#[cfg(...)]` attributes for conditional compilation:
58+
59+
```rust
60+
#[cfg(windows)]
61+
use pet_windows_registry::WindowsRegistry;
62+
63+
#[cfg(unix)]
64+
fn resolve_path(p: &Path) -> PathBuf { std::fs::canonicalize(p).unwrap_or(p.to_path_buf()) }
65+
66+
#[cfg(target_os = "macos")]
67+
use pet_homebrew::Homebrew;
68+
```
69+
70+
**Do NOT use `if cfg!(windows) { ... }` for code that imports platform-specific modules** — the code inside still gets compiled on all platforms.
71+
72+
### Platform Gotchas:
73+
- **Windows:** Avoid `canonicalize()` for directory junctions (breaks Scoop symlinks). Use `norm_case()` instead.
74+
- **macOS:** Homebrew has complex symlink chains; `resolve_symlink` before `canonicalize`
75+
- **Linux:** `/bin` may symlink to `/usr/bin` — handle both
76+
77+
## Version Detection (Prefer File-Based)
78+
79+
**Order of preference (cheapest first):**
80+
1. `pyvenv.cfg` — `version` or `version_info` field
81+
2. `conda-meta/python-*.json` — package metadata
82+
3. `Include/patchlevel.h` — CPython source header
83+
4. Parse from executable path (e.g., `python3.11`)
84+
5. Spawn Python `--version` — **LAST RESORT** (expensive, defeats PET's purpose)
85+
86+
## Path & Symlink Handling
87+
88+
- Use `pet_fs::path::resolve_symlink()` — the project's own resolver
89+
- For comparison: `canonicalize().unwrap_or(original)` then `norm_case()` for Windows UNC prefix handling
90+
- Preserve original user-facing paths; resolve only for identification
91+
92+
## Testing Conventions
93+
94+
- Feature-gated CI tests: `#[cfg_attr(feature = "ci", test)]` with `#[allow(dead_code)]`
95+
- Feature flags: `ci`, `ci-poetry-global`, `ci-jupyter-container`, `ci-homebrew-container`
96+
- Verification pattern: spawn Python, use `try_from()`, test symlinks, use `resolve`
97+
- Use `cargo test --features ci` for CI-gated tests
98+
99+
## Serialization
100+
101+
- All JSON-serialized structs use `#[serde(rename_all = "camelCase")]`
102+
- Matches the JSONRPC API conventions consumed by VS Code Python extension
103+
104+
## Pre-Commit Requirements
105+
106+
Always run before committing:
107+
```bash
108+
cargo fmt --all
109+
cargo clippy --all -- -D warnings
110+
```
111+
Never suppress clippy warnings with `#[allow(...)]` without justification.
112+
113+
## Conda-Specific Patterns
114+
115+
- Conda manager detection takes precedence over mamba: `get_conda_manager(path).or_else(|| get_mamba_manager(path))`
116+
- Managers reported separately (Conda, Mamba) but environments remain `PythonEnvironmentKind::Conda`
117+
- Support detection from history files and conda-meta directories

0 commit comments

Comments
 (0)