diff --git a/.codex/skills/pr-fixes/SKILL.md b/.codex/skills/pr-fixes/SKILL.md new file mode 100644 index 0000000..9e7d79a --- /dev/null +++ b/.codex/skills/pr-fixes/SKILL.md @@ -0,0 +1,163 @@ +--- +description: "Address PR review comments, make fixes, reply, and resolve" +argument-hint: "" +model: claude-opus-4-5-20251101 +--- + +# Address PR Review Comments + +Automates addressing pull request review feedback: analyze comments, make fixes, reply to explain changes, resolve threads, and request re-review from @codex. + +## Prerequisites + +- Must have `gh` CLI authenticated +- Must be in a git repository with a GitHub remote + +## Step 1: Validate Input + +**If $ARGUMENTS is empty:** +- Use AskUserQuestion to ask for the PR number +- Validate it's a number + +**If $ARGUMENTS is provided:** +- Extract PR number from $ARGUMENTS +- Validate it's a valid number + +## Step 2: Fetch PR Review Threads + +Run this GraphQL query to get all unresolved review threads: + +```bash +gh api graphql -f query=' +query { + repository(owner: "{owner}", name: "{repo}") { + pullRequest(number: PR_NUMBER) { + id + reviewThreads(first: 100) { + nodes { + id + isResolved + path + line + comments(first: 10) { + nodes { + id + databaseId + body + author { + login + } + } + } + } + } + } + } +}' +``` + +Filter to only unresolved threads (`isResolved: false`). + +If there are no unresolved threads, inform the user and exit. + +## Step 3: Analyze and Categorize Comments + +For each unresolved thread: + +1. Read the comment body to understand the feedback +2. Identify the file path and line number +3. Read the relevant file section to understand the context +4. Categorize the type of feedback: + - **Code change needed** - requires file modification + - **Documentation** - needs comment/doc update + - **Question** - requires explanation only, no code change + - **Disagree/Won't fix** - ASK USER before responding + +Present a summary to the user showing: +- Number of comments found +- File paths affected +- Brief description of each comment + +**IMPORTANT**: For any comment where you disagree or think "won't fix" is appropriate, use AskUserQuestion to get user confirmation before replying. Never auto-resolve disagreements. + +## Step 4: Address Each Comment + +For each comment requiring action: + +### 4a. Read the relevant file + +Use the Read tool to understand the context around the specified line. + +### 4b. Make the fix + +Use the Edit tool to make the necessary changes based on the feedback. + +### 4c. Reply to the comment + +Use REST API to reply to the comment explaining what was done: + +```bash +gh api --method POST \ + repos/{owner}/{repo}/pulls/PR_NUMBER/comments/COMMENT_DB_ID/replies \ + -f body='Fixed: [explanation of what was changed and why]' +``` + +Keep replies concise but informative. Explain WHAT was changed and WHY. + +### 4d. Resolve the thread + +Use GraphQL mutation to resolve the thread: + +```bash +gh api graphql -f query=' +mutation { + resolveReviewThread(input: {threadId: "THREAD_NODE_ID"}) { + thread { isResolved } + } +}' +``` + +## Step 5: Commit Changes + +After all fixes are made: + +1. Stage all changed files with `git add` +2. Create a commit with a descriptive message following conventional commits format: + +```bash +git commit -m "fix: address PR review feedback + +- [list each fix made] +- [reference comment IDs if helpful] + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +Co-Authored-By: Claude Opus 4.5 " +``` + +3. Push the changes to the remote branch + +## Step 6: Request Re-Review + +Add a comment to the PR requesting re-review from @codex: + +```bash +gh pr comment PR_NUMBER --body '@codex review' +``` + +## Step 7: Summary + +Present a summary to the user: + +- Number of comments addressed +- Files modified +- Commit hash created +- Any comments that were NOT addressed (with reasons) +- Link to the PR + +## Error Handling + +- If `gh` CLI is not authenticated, instruct user to run `gh auth login` +- If PR number is invalid, show error and ask for correct number +- If a thread fails to resolve, log the error but continue with other threads +- If commit fails, show the error and suggest manual resolution \ No newline at end of file diff --git a/src/main.rs b/src/main.rs index f694e79..45080b0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -210,12 +210,24 @@ fn try_main() -> anyhow::Result<()> { // [2]: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/kqueue.2.html unsafe { signal(Signal::SIGINT, SigHandler::SigIgn) }?; - let mut args: Args = Args::parse(); + let Args { + vm_fd, + vm_mac_address, + vm_net_type, + bootpd_lease_time, + user, + group, + allow, + block, + expose, + sudo_escalation_probing, + sudo_escalation_done, + } = Args::parse(); // No need to run anything, just return // so that the invoker process knows we // can be invoked in Sudo as root - if args.sudo_escalation_probing { + if sudo_escalation_probing { return Ok(()); } @@ -231,7 +243,7 @@ fn try_main() -> anyhow::Result<()> { // Ensure we are running as root if get_effective_uid() != 0 { - if sudo_escalation_works() && !args.sudo_escalation_done { + if sudo_escalation_works() && !sudo_escalation_done { let exe = std::env::current_exe().unwrap(); let args = std::env::args().skip(1); @@ -253,28 +265,28 @@ fn try_main() -> anyhow::Result<()> { )); } - let allow = resolve_allow_block_entries("allow", std::mem::take(&mut args.allow))?; - let block = resolve_allow_block_entries("block", std::mem::take(&mut args.block))?; + let allow = resolve_allow_block_entries("allow", allow)?; + let block = resolve_allow_block_entries("block", block)?; // Set bootpd(8) min/max lease time while still having the root privileges - set_bootpd_lease_time(args.bootpd_lease_time); + set_bootpd_lease_time(bootpd_lease_time); // Initialize the proxy while still having the root privileges let mut proxy = Proxy::new( - args.vm_fd as RawFd, - args.vm_mac_address, - args.vm_net_type, + vm_fd as RawFd, + vm_mac_address, + vm_net_type, PrefixSet::from_iter(allow), PrefixSet::from_iter(block), - args.expose, + expose, ) .context("failed to initialize proxy")?; // Drop effective privileges to the user // and group which have had invoked us PrivDrop::default() - .user(args.user.unwrap_or(current_user_name)) - .group(args.group.unwrap_or(current_group_name)) + .user(user.unwrap_or(current_user_name)) + .group(group.unwrap_or(current_group_name)) .apply() .context("failed to drop privileges")?; @@ -325,15 +337,16 @@ mod tests { use std::net::Ipv4Addr; #[test] - fn resolve_domain_to_ipv4_nets_example_com() { + fn resolve_domain_to_ipv4_nets_dns_google() { let nets = resolve_allow_block_entries( "allow", - vec![AllowBlockEntry::Domain("example.com".to_string())], + vec![AllowBlockEntry::Domain("dns.google".to_string())], ) .unwrap(); - assert!(nets.contains(&Ipv4Net::from(Ipv4Addr::new( - 93, 184, 216, 34 - )))); + assert!( + nets.contains(&Ipv4Net::from(Ipv4Addr::new(8, 8, 8, 8))) + || nets.contains(&Ipv4Net::from(Ipv4Addr::new(8, 8, 4, 4))) + ); } }