Github Contributor

GitHub Contributor

A phase-based playbook for shipping pull requests that maintainers actually want to merge. The skill is structured around the real PR lifecycle — discovery → implementation → quality gates → description → post-submission — because each phase has its own failure modes and the most common mistake is doing the right thing at the wrong phase (e.g., writing the perfect description for a PR that's 10× too large).

Phase 0 — When to use this skill

Use this skill when all of these are true:

• You are contributing to a repo you do not maintain (the maintainer can close your PR without explanation).
• The work touches one or more of: source code, tests, docs, build config.
• You want the PR merged, not just submitted.

Do not use this for: your own repos, internal team PRs with shared context, hot-fix branches where a maintainer is waiting on you, or trivial single-line changes (one comment is enough).

Phase 1 — Pre-PR Discovery

The most common reason PRs get closed is a mismatch between what the contributor assumes is acceptable and what the maintainer has already written down. Solve this before writing code.

Step 1.1 — Read CONTRIBUTING.md as a hard contract

CONTRIBUTING.md is not style advice. Treat every numbered rule as a precondition for merge. Pay special attention to:

• AI-assisted contribution clauses. Many projects added these in 2024-2026 after the AI PR wave. Typical phrasing: "AI-generated PRs without prior discussion may be closed", "you must be able to explain every line", "one issue, one PR". If this clause exists, you owe the project explicit disclosure (see Phase 4) and you must keep the PR small.
• Issue-first rules. Some projects require a feature-request issue to exist before any feature PR is opened.
• Per-language test commands. If CONTRIBUTING.md says pnpm test:unit && cargo test, those are the commands you run, not whatever your IDE prefers.

If CONTRIBUTING.md is missing, that itself is a red flag — see `references/project_evaluation.md`.

Step 1.2 — Sanity-check your PR size against the project's baseline

A "small PR" is relative. Before opening a PR, run:

gh pr list --repo <owner>/<repo> --state merged --limit 10 \
  --json number,title,author,additions,deletions \
  --jq '.[] | "#\(.number) +\(.additions)/-\(.deletions): \(.title)"'

This tells you the project's actual merged-PR size distribution. If your PR is 5–10× larger than the biggest recent merge, that is a red signal — split before submitting. See `references/phase1_discovery.md` for the baseline rubric and split heuristics.

Step 1.3 — Write a one-paragraph scope contract before coding

A scope contract is a single paragraph you write to yourself before opening your editor:

Goal: <one sentence>. In scope: <bullet list, 3–5 items>. Explicitly out of scope: <bullet list — be specific about what you will resist adding when it's tempting>.

Then, every time you make an edit, ask: "Is this in scope?" If you find yourself "while I'm in here…"-ing, stop and revisit the contract. Scope creep is the single biggest source of close-without-merge — see `references/phase2_implementation.md` for the scope-discipline section.

Phase 2 — Implementation

Step 2.1 — Branch off main immediately after fetching upstream

git fetch origin
git switch -c feat/short-descriptive-name origin/main

Always branch from upstream main (or the project's default branch), never from your fork's main, which may be stale.

Step 2.2 — Make the smallest diff that solves the problem

Resist any change that is not directly required by your scope contract. In particular:

• Do not "while I'm here" refactor surrounding code.
• Do not reformat lines you didn't touch (your formatter may differ from the project's, even if both say "Prettier").
• Do not rename variables for clarity unless the renaming is the fix.

If a follow-up improvement is genuinely valuable, file a separate issue or open a separate PR after this one is merged.

Step 2.3 — Conventional Commits, one logical change per commit

Use Conventional Commits: <type>(<scope>): <description> where type is feat | fix | docs | refactor | test | chore | ci | perf. Each commit should be reviewable on its own.

When a review prompts a fix, use git commit --fixup=<sha> and squash with git -c sequence.editor=: rebase -i --autosquash origin/main before pushing — see `references/phase2_implementation.md` for the full fixup workflow.

Phase 3 — Quality Gates

Maintainers' trust is built by evidence, not by claims. The point of this phase is to produce evidence you can paste into the PR.

Step 3.1 — Run the project's full lint + test suite locally

Read the exact commands from CONTRIBUTING.md. Typical examples (use what your project specifies):

pnpm typecheck && pnpm format:check && pnpm test:unit
cargo fmt --check && cargo clippy --all-targets && cargo test

If any check fails, fix it before continuing. Do not push a PR with red local checks expecting CI to clarify — that wastes maintainer time.

Step 3.2 — For GUI / desktop apps: run real end-to-end with isolation

For Tauri/Electron/Cocoa apps you almost certainly cannot use pnpm dev directly without contaminating your real installation. The pattern is isolate the data directory first, then run the real binary:

1. Find the project's test-isolation hook (often XXX_TEST_HOME, XXX_DATA_DIR, or a config flag in config.rs / paths.go). 2. Point it at /tmp/<app-name>-e2e/ before launching. 3. Trigger the feature through whatever real surface the user would (URL scheme, CLI arg, deeplink). 4. Verify by reading the actual persisted state (SQLite, JSON files), not just by visual inspection. 5. Capture screenshots of the GUI for the PR description.

The full isolation recipe, including how to trigger deeplinks via Tauri's single-instance forward without touching macOS LaunchServices, is in `references/phase3_quality_gates_and_e2e.md`.

Step 3.3 — Self-audit: did you actually do what you're about to claim?

Before writing the PR description, list every "I tested…" / "I verified…" / "I ran…" statement you intend to make. For each one, ask: "What's my evidence?" If the answer is "I think I did" or "it should work", you have not actually done it. Write only what you can defend.

This rule prevents the most damaging trust failure: a maintainer running your "tested" command and finding it doesn't work.

Phase 4 — PR Description Writing

A great PR description does three jobs: (1) lets the maintainer decide in 30 seconds whether to merge, (2) gives reviewers everything they need to verify without DM'ing you, (3) creates a written record that survives team turnover.

Step 4.1 — Structure

Use this skeleton. Detailed templates and a test-coverage-matrix example are in `references/phase4_pr_description.md` and `references/communication_templates.md`.

## Summary / 概述
<two sentences — what changed and why it matters>

## What / 变更内容
<bulleted list of commits with their purpose, or files with their purpose>

## Why / 动机
<the problem this solves; if no prior issue, briefly justify why>

## Test Plan / 测试计划
<exact commands a maintainer can run; coverage matrix for non-trivial changes>

## Backward Compatibility / 向后兼容
<state explicitly; don't make the maintainer infer>

## Security Considerations
<only if the change touches auth, inputs, or shared state>

## Screenshots / 截图
<for UI changes — see Step 4.3>

## Related Issue
<Fixes #N, or explain why no issue exists>

## Checklist
<the project's PR template checklist, with real evidence of each>

## AI-Assisted Disclosure
<see Step 4.4>

Step 4.2 — Test coverage matrix (for non-trivial changes)

When you've added more than 2 tests, present them as a table mapping each test to the behavior it locks in. This makes review much faster than reading test code:

| Layer | Test | What it proves |
|---|---|---|
| URL parsing | `test_parse_provider_with_extra_env` | extraEnv query param extracted |
| Security | `test_extra_env_stringifies_scalars_and_skips_invalid_values` | bool/number stringified; null/array/object dropped |

Step 4.3 — Screenshots without polluting the repo

gh CLI does not support image attachments to PRs (the underlying upload API at uploads.github.com is browser-only and rejects PAT tokens). Three workable approaches:

1. Preferred — let the user drag images in the GitHub web UI. Leave clearly marked placeholders in your PR body draft (e.g. [SCREENSHOT_1_PLACEHOLDER]). When the user edits the PR on github.com, they drag images into the markdown, GitHub uploads them to user-images.githubusercontent.com, and the placeholders are replaced. Zero pollution. 2. Fallback — orphan branch on your fork. Create an orphan branch (e.g. named assets-pr-N-screenshots), commit images, reference them via raw.githubusercontent.com. Pollutes your fork but not the PR diff. 3. Last resort — third-party image host. Persistence + privacy are unclear; avoid for anything sensitive.

Step 4.4 — AI-Assisted Disclosure (when CONTRIBUTING.md or maintainer norms call for it)

If the project's CONTRIBUTING.md mentions AI-assisted PRs, or the maintainer has commented skeptically about AI output on past PRs, add a short disclosure at the bottom of the PR body. Be specific about what you did, not vague reassurances.

## AI-Assisted Disclosure

Per CONTRIBUTING.md §N:

1. I have read every line; happy to walk through any function or design choice.
2. Tested locally: <list actual commands you ran with their results>.
3. Single-topic PR scoped to <one sentence>.
4. <opened/will open> Issue #N for discussion.
5. AI tools used: Claude Code for drafting; <list any others>. Final review and decisions are mine.

The disclosure is not magic — it doesn't excuse a bad PR. But missing it on a project that asks for it is an instant trust hit.

Phase 5 — Post-Submission

Step 5.1 — Respond to automated bot reviews explicitly

Modern projects use Codex, Claude bot, CodeRabbit, etc. for first-pass review. Their comments appear as review comments on specific lines, not as PR-level comments. Reply to each finding directly (so maintainers see the resolution next to the finding), citing the commit hash and the function/test that resolves it:

gh api repos/<owner>/<repo>/pulls/<pr>/comments \
  -X POST \
  -F in_reply_to=<finding_comment_id> \
  -f body="Addressed in commit \`<sha>\`: <function or test name>. <one-sentence explanation>. Thanks for the catch!"

<finding_comment_id> is the numeric ID from the comment's URL (#discussion_rXXXXXXXX). Full bot-reply workflow in `references/phase5_post_submission.md`.

Step 5.2 — Rebase against upstream main without losing review history

When upstream main advances and your PR conflicts:

git fetch origin
git rebase origin/main
# resolve conflicts file by file
git add <files>
git -c sequence.editor=: rebase --continue
git push fork <branch> --force-with-lease

Use --force-with-lease, never plain --force. The lease variant aborts if someone else (or a bot) pushed to your branch in between, which prevents you from silently destroying review threads.

If you applied a small post-review cleanup (a --fixup commit), squash it into the relevant commit with autosquash so the merged history stays clean. See `references/phase2_implementation.md` for the full sequence.

Step 5.3 — When sub-agent / counter-review surfaces "findings", filter before responding

If you run a counter-review agent (or a maintainer's bot floods you with 20+ findings), don't paste them all into the PR. For each finding ask three questions:

The point of counter-review is to surface things you didn't think of, not to mandate fixing every theoretical concern. Filter ruthlessly, then explain in the PR why you accepted vs. declined each suggestion.

Reference Files

Anti-Patterns to Avoid

These are the failure modes that close PRs even when the underlying code is fine. Each one comes from a real PR.

1. Fabricated test claims. Writing "tested locally with pnpm dev" when you actually only ran the unit tests. A maintainer will try it and lose trust permanently. 2. PR 5–10× the project's recent merge baseline. Even good code at this size signals "AI dump" to many maintainers. 3. Rebase-time scope creep. Bringing an unrelated upstream feature into your branch "while resolving conflicts" turns a fix PR into a feature PR with no warning. 4. Mixing refactors into a fix commit. Reviewers can't tell which line caused the bug fix; either split or use a --fixup commit on the refactor. 5. Force-pushing without `--lease` mid-review. Destroys review threads silently. 6. Ignoring bot review comments. Even when the bot is wrong, reply explaining why — silence reads as "didn't notice". 7. Burying the disclosure. AI-assisted disclosure goes in the PR body, not as a footnote in a commit message no one reads. 8. Reading CONTRIBUTING.md after writing the PR. Half of CONTRIBUTING.md rules are about how the PR is structured, not what the code does. 9. Submitting features without an issue when the project requires one. Even if the issue is created retroactively the same hour, the timestamp matters to maintainers. 10. Pasting raw counter-review output. 20 findings in a PR body looks like noise. Filter, then respond.

Quick Reference

Required gh CLI commands

gh repo view <owner>/<repo> --json visibility,isPrivate,defaultBranchRef
gh pr list --repo <owner>/<repo> --state merged --limit 10
gh pr view <pr-number> --repo <owner>/<repo> --json title,body,commits,mergeable,reviewDecision
gh pr edit <pr-number> --repo <owner>/<repo> --body-file pr_body.md
gh api repos/<owner>/<repo>/pulls/<pr>/comments -X POST -F in_reply_to=<id> -f body="..."

Conventional Commits cheat sheet

feat(<scope>): user-visible new behavior
fix(<scope>):  user-visible bug fix
refactor(<scope>): no behavior change
docs(<scope>): documentation only
test(<scope>): tests only
chore(<scope>): tooling / build / housekeeping
perf(<scope>): measurable performance change
ci(<scope>): CI config only

Key metrics for a high-quality PR

Based on successful contributions to active projects:

• Files changed: 1-5 for fixes, up to ~15 for features with tests
• Production code diff: under 200 lines if possible; rest is tests / docs
• PR description: 200-600 lines including evidence; matrix tables welcome
• First-response time to bot/maintainer: under 24h
• CI passing on first push: target

If your PR misses two or more of these by a lot, re-read Phase 1 before submitting.

Case Study: cc-switch PR #2634 (extraEnv support for deeplinks)

A complete walk-through of a real PR submitted to farion1231/cc-switch (a Tauri + React desktop app for switching Claude/Codex/Gemini providers). The PR adds an extraEnv parameter to the project's ccswitch:// deeplink import flow, allowing distributors to ship UI-toggle settings inside the deeplink URL.

This case is preserved as a reference because it touches every phase of the playbook and includes failure modes that the rest of the skill explicitly warns against (fabrication near-miss, scope creep at rebase time, isolated GUI E2E with hardcoded paths).

PR URL: https://github.com/farion1231/cc-switch/pull/2634

Phase 1 findings that shaped the PR

CONTRIBUTING.md AI-Assisted clause (most important finding)

The project's CONTRIBUTING.md ends with a five-rule AI-Assisted Contributions section. Verbatim summary:

1. You have read and understood your code. You must be able to explain any line. 2. You have tested it yourself. No "looks right". 3. One issue, one PR. Sprawling multi-topic PRs are closed. 4. Open an issue first. Drive-by PRs may be closed. 5. Maintainers may close without explanation. Hallucinated fixes, unnecessary refactors, bulk changes get closed.

Discovering this clause changed the entire PR-writing approach: every claim had to be specifically verifiable, the disclosure block became non-optional, and any temptation to "while I'm here" refactor had to be resisted.

PR-size baseline check

gh pr list --repo farion1231/cc-switch --state merged --limit 10 \
  --json number,title,additions,deletions

Output (the 10 most recently merged PRs at the time):

Largest recent merge was +190/-4. Our PR ended at +1103/-26. That's roughly 5–10× the project's normal merge size — a red signal we acknowledged in the PR body upfront and offered to split if the maintainer preferred.

Phase 2 implementation choices

Scope contract

Goal: support a Base64-encoded JSON extraEnv parameter in ccswitch:// deeplinks for Claude and Gemini providers, so distributors can pre-set environment variables that are otherwise UI-only.

>

In scope: parsing the new query param; merging values into settings_config.env; validation/sanitization of injected keys; unit + integration tests; demo update; CHANGELOG entry.

>

Explicitly out of scope: changing the deeplink scheme; touching providers other than Claude/Gemini; refactoring the existing deeplink parser; UI changes beyond the demo HTML page.

Two-commit structure

The PR landed as exactly two commits, separated by Codex's automated review:

1. feat(deeplink): support extraEnv parameter for provider configuration — added the parameter, merge logic, four tests. 2. fix(deeplink): harden extraEnv import behavior — addressed Codex's P1+P2 review findings (described below). The second commit also picked up a small unrelated code-simplifier cleanup; this almost violated the scope contract and should have been split out — see "Lessons" below.

Rebase-time conflict

While the PR was open, upstream main landed a separate "ClaudeDesktop provider" feature that touched the same provider.rs file. The rebase produced two conflicts in build_provider_from_request:

• An import line (use crate::provider::...) had grown a new symbol upstream.
• A match app_type block had grown a new ClaudeDesktop arm upstream.

The conflict was resolved by extending our extraEnv support to also cover ClaudeDesktop (since the two providers share the same build_claude_settings function). This was technically scope creep — the original scope contract said "Claude and Gemini" — but was unavoidable given the file-level overlap. The PR description was updated to acknowledge the additional coverage.

Lesson: when rebase forces scope extension, declare it in the PR body. Don't let the maintainer discover it.

Phase 3 — Isolated GUI end-to-end verification

This is the part the rest of the playbook references most often, because cc-switch hardcodes its data directory:

// src-tauri/src/config.rs:95
let default_dir = get_home_dir().join(".cc-switch");

Changing the Tauri identifier or CFBundleURLSchemes is therefore not enough to isolate from production data. The project does, however, provide a test hook:

// src-tauri/src/config.rs:23
pub fn get_home_dir() -> PathBuf {
    if let Ok(home) = std::env::var("CC_SWITCH_TEST_HOME") {
        let trimmed = home.trim();
        if !trimmed.is_empty() {
            return PathBuf::from(trimmed);
        }
    }
    dirs::home_dir().unwrap_or_else(|| { ... })
}

Isolation recipe

# 1. Pre-emptive backup in case anything leaks
cp -a ~/.cc-switch ~/.cc-switch.backup-pre-e2e-$(date +%s)

# 2. Build the dev binary (one-time)
pnpm tauri dev &
# (kill the auto-launched window; we just want the binary built)
pkill -f 'tauri dev'

# 3. Re-launch with isolated home
mkdir -p /tmp/cc-switch-e2e/.cc-switch
CC_SWITCH_TEST_HOME=/tmp/cc-switch-e2e pnpm tauri dev &

# 4. Trigger the deeplink via single-instance forward (does NOT use macOS LaunchServices)
CC_SWITCH_TEST_HOME=/tmp/cc-switch-e2e \
  ./src-tauri/target/debug/cc-switch "ccswitch://v1/import?resource=provider&app=claude&..."

The fourth step is the part that bypasses macOS scheme handler registration. Tauri 2 ships a single_instance plugin: when you launch the binary a second time with a URL as argv[1], the running instance receives it through the single-instance callback. This is the cleanest way to test deeplinks on dev binaries (which aren't real .app bundles and therefore can't register URL schemes).

Verification matrix actually run

The test payload had 10 extraEnv keys, four of which were designed to trigger Codex's P1/P2 hardening:

Real dev-log captured (redacted)

[INFO] === Single Instance Callback Triggered ===
[INFO] ✓ Deep link URL detected from single_instance args: ccswitch://v1/import?[keys:apiKey,app,enabled,endpoint,extraEnv,model,name,resource]
[INFO] ✓ Successfully parsed deep link: resource=provider, app=Some("claude"), name=Some("e2e-test-extraenv")
[INFO] ✓ Emitted deeplink-import event to frontend
[INFO] Importing provider resource from deep link
[WARN] Skipping extra_env key 'ANTHROPIC_AUTH_TOKEN': protected env fields must be non-empty strings
[WARN] Skipping extra_env key 'CLAUDE_CODE_BAD_OBJECT': arrays/objects are not valid env values
[INFO] Provider 'e2e-test-extraenv-1778611889499' set as current for Claude

SQLite verification

After import, query the isolated database directly:

sqlite3 /tmp/cc-switch-e2e/.cc-switch/cc-switch.db \
  "SELECT settings_config FROM providers WHERE name='e2e-test-extraenv'" | \
  python3 -c "import json,sys; print(json.dumps(json.loads(sys.stdin.read()), indent=2))"

Result confirmed all 11 expected behaviors (6 strings preserved, 2 scalars stringified, null-override dropped, object dropped).

Cleanup

pkill -f 'target/debug/cc-switch'
git checkout HEAD -- src-tauri/tauri.conf.json src-tauri/Info.plist  # in case configs were touched
rm -rf /tmp/cc-switch-e2e
# Keep the backup ~/.cc-switch.backup-pre-e2e-* for a few days, then delete

Phase 4 — PR description structure used

The actual body that was submitted (paraphrased headers):

## Summary / 概述
## What / 变更内容          ← two commits with their roles
## Why / 动机
## Test Plan / 测试计划      ← coverage matrix of 21 tests
## How to verify locally    ← exact reproducible commands
## Backward Compatibility / 向后兼容
## Security Considerations  ← references Codex P1/P2 findings + how they were fixed
## Screenshots / 截图        ← with placeholder text the user replaced via drag-and-drop
## Related Issue            ← "no prior issue; happy to retro-file if preferred"
## Checklist / 检查清单      ← each box with actual evidence
## AI-Assisted Disclosure

The description landed at roughly 600 lines including evidence and matrices. This is large relative to the production diff but defensible because most of it is evidence, not narration.

Phase 5 — Bot review handling

Codex left two review comments on the first commit:

• P1: ANTHROPIC_AUTH_TOKEN could be overwritten by extraEnv with null/non-string.
• P2: merge_extra_env should validate value types.

Both were addressed in commit 2 (fix(deeplink): harden extraEnv import behavior). Replies were posted directly under each finding via the GitHub API:

gh api repos/farion1231/cc-switch/pulls/2634/comments \
  -X POST \
  -F in_reply_to=3225389962 \
  -f body="Addressed in commit \`bade3de1\`: \`is_protected_env_key\` now blocks non-string overrides of protected keys. \`normalize_extra_env_value\` enforces the type discipline. Regression locked in by \`test_extra_env_stringifies_scalars_and_skips_invalid_values\`. Thanks for the catch!"

Replying as a comment (not in PR body) means the resolution appears next to the finding for any future reviewer.

Lessons that became skill rules

1. CONTRIBUTING.md `AI-Assisted` clauses are merge gates — they shaped the entire PR strategy. 2. PR size sanity check before submission catches "too big" before the maintainer has to point it out. 3. Isolated GUI E2E with the project's own test hook beats trying to override identifier / scheme. 4. Fabrication near-miss: the first draft of the PR body said "tested with pnpm dev and deplink.html flow" — but the manual GUI flow had not actually been run yet. Caught during self-audit; rewritten to list only what was real. This is now a Phase 3 rule. 5. Rebase-time scope creep should be acknowledged in the body, not hidden. 6. Bot reply via `gh api in_reply_to` keeps the resolution next to the finding. 7. Screenshot placeholders > image hosting hacks. The clean path is to leave [SCREENSHOT_N_PLACEHOLDER] in the PR body and let the user drag images into the GitHub web editor. 8. `code-simplifier` cleanups should be a separate commit (or a separate PR) — bundling them into a fix commit makes review harder and risks scope creep. 9. `--force-with-lease`, never plain `--force` — review threads are too easy to destroy. 10. Self-audit "what's my evidence?" pass before publishing the PR body catches fabrication-by-default.

Communication Templates

Templates for effective open-source communication.

Claiming an Issue

First-Time Contributor

Hi! I'm interested in working on this issue.

I'm new to the project but I've read the contributing guidelines and set up the development environment. I think I understand the scope of the change needed.

My approach would be to:
1. [Step 1]
2. [Step 2]
3. [Step 3]

Does this sound reasonable? Any guidance would be appreciated!

Experienced Contributor

I'd like to take this on.

Proposed approach:
- [Technical approach]
- [Testing strategy]

ETA: [timeframe]

Let me know if there are any concerns or if someone else is already working on this.

Asking for Clarification

Thanks for filing this issue!

I'd like to work on this but need some clarification:

1. [Question 1]
2. [Question 2]

Once I understand these points, I can start on a fix.

PR Description

Bug Fix

## Summary

Fixes #[issue number]

This PR resolves the [bug description] by [solution approach].

## Root Cause

The issue was caused by [explanation].

## Solution

[Detailed explanation of the fix]

## Testing

- [x] Added regression test
- [x] Verified fix locally
- [x] All existing tests pass

## Screenshots (if applicable)

Before:
[image]

After:
[image]

Feature Addition

````markdown

Summary

Implements #[issue number]

Adds [feature description] to enable [use case].

Changes

• Added feature.py with [functionality]
• Updated config.py to support [new option]
• Added tests in test_feature.py

Usage

# Example usage
from project import new_feature
result = new_feature(...)

Testing

• [x] Unit tests added
• [x] Integration tests pass
• [x] Documentation updated

Migration Guide (if breaking)

[Instructions for users to migrate] ````

Documentation Update

## Summary

Improves documentation for [area].

## Changes

- Fixed typos in [file]
- Added examples for [feature]
- Updated outdated [section]
- Clarified [confusing part]

## Preview

[Screenshot or link to rendered docs]

Required PR Addendum

Use this block in every PR description.

````markdown

Evidence Loop

Command:

# Baseline (before fix)
[command]

# Fixed (after fix, same command)
[command]

Raw output:

[baseline output]

[fixed output]

Comparison (Baseline vs Fixed vs Reference)

Sources/Attribution

• [Issue, docs, benchmark source, or code reference]

Risks

• [Risk and impact]

Rollback Plan

• Revert commit(s): [hash]
• Restore previous behavior with: [command]

````

Reproducible PR Comment

Use this template when maintainers ask for proof or rerun details.

````markdown Validated with the same command before and after the fix.

Command

[command]

Environment

• OS: [name/version]
• Runtime: [language/runtime + version]
• Commit: [sha]
• Runner: [local shell or CI job URL]

Baseline Output (before fix)

[raw output]

Fixed Output (after fix, same command)

[raw output]

Reference Output / Expected Behavior

[spec output or expected result]

Redaction Check

• [x] Removed local absolute paths (for example, /Users/...)
• [x] Removed tokens/secrets
• [x] Removed internal URLs/hostnames

````

Responding to Reviews

Accepting Feedback

Good catch! I've updated the code to [change].

See commit [hash].

Explaining a Decision

Thanks for the review!

I chose this approach because:
1. [Reason 1]
2. [Reason 2]

However, I'm open to changing it if you think [alternative] would be better. What do you think?

Requesting Clarification

Thanks for the feedback!

Could you clarify what you mean by [quote]? I want to make sure I address your concern correctly.

Disagreeing Respectfully

I see your point about [concern].

I went with the current approach because [reasoning]. However, I understand the tradeoff you're highlighting.

Would a middle ground like [alternative] address your concern while keeping [benefit]?

After Merge

Thanks for the review and merge! 🎉

I learned [something] from the feedback - I'll apply that in future contributions.

Looking forward to contributing more to the project!

Abandoning a PR

Hi, I won't be able to complete this PR due to [reason].

I've pushed my current progress in case someone else wants to continue from here. The remaining work is:
- [ ] [Task 1]
- [ ] [Task 2]

Sorry for any inconvenience, and thanks for the opportunity to contribute!

Tone Guidelines

Always

• Be grateful
• Be specific
• Be patient
• Be humble

Never

• Be defensive
• Be dismissive
• Be demanding
• Be passive-aggressive

Word Choice

❌ "You should..."
✅ "It might help to..."

❌ "This is wrong"
✅ "I think there might be an issue with..."

❌ "Obviously..."
✅ "One approach could be..."

❌ "Why didn't you..."
✅ "Could you help me understand..."

High-Quality PR: Real-World Case Study

Based on OpenClaw PR #39763 - A successful bug fix contribution to a 278K star TypeScript project.

What Made This PR High-Quality

1. Complete Evidence Chain

Issue → Root Cause → Fix → Validation

✅ Original bug report with symptoms
✅ Deep investigation with timeline analysis
✅ Root cause identified in source code
✅ Minimal, surgical fix
✅ End-to-end testing with before/after comparison
✅ Regression test added

2. Thorough Investigation Before Coding

Timeline Analysis (Posted to issue, not PR):

• Traced bug through 3 years of related changes
• Identified when workaround was added (#29078)
• Explained why removing workaround is now safe
• Linked to all relevant historical PRs and issues

Key insight: Detailed investigation goes in the issue, not the PR. Keep PR focused on the fix.

3. Minimal, Focused Changes

Files changed: 2

• src/infra/process-respawn.ts - 3 lines removed, 1 line added
• src/infra/process-respawn.test.ts - Updated tests + regression test

What we didn't do:

• ❌ Refactor surrounding code
• ❌ Add "improvements" beyond the fix
• ❌ Change unrelated files
• ❌ Add extensive comments

4. Regression Test Added

test("launchd path never returns failed status", () => {
  const result = detectSupervisor("launchd");
  expect(result.mode).not.toBe("failed");
  expect(result.mode).toBe("supervised");
});

Why this matters: Prevents the bug from being reintroduced.

5. CHANGELOG Entry

Following project conventions:

## [Unreleased]

### Fixed
- **darwin/launchd**: Remove `kickstart -k` self-restart to prevent race condition with launchd bootout (#39763)

Key: Check if project maintains CHANGELOG and follow their format exactly.

6. Clear PR Structure

Title: fix(darwin): remove launchd kickstart race condition

• Conventional commit format
• Scope indicates platform
• Clear what was fixed

Body (~50 lines, trimmed from original 136):

## Summary
[2 sentences: what + why]

## Root Cause
[Technical explanation with code references]

## Changes
- [Bullet list of actual changes]

## Why This Is Safe
[Explain why the fix won't break anything]

## Testing
[How it was validated]

## Related
- Fixes #39760
- Related: #27650, #29078

7. Separation of Concerns

Issue comment: Detailed timeline, investigation, evidence PR description: Focused on the fix, testing, safety Separate test comment: End-to-end validation results

Why: Keeps PR reviewable. Detailed context available but not blocking review.

8. End-to-End Testing

Test 1: Reproduced bug with original version

Result: Bootstrap failed: 5, SIGKILL, exit code -9 ✅

Test 2: Validated fix with patched version

Result: Clean restart, no errors, normal exit code ✅

Evidence: Posted full logs with timestamps, PIDs, exit codes.

9. What We Avoided

❌ Don't mention internal tooling:

• We had a custom monitor script that auto-remediated the bug
• We initially mentioned it in PR comments
• Removed it because it's not part of OpenClaw - would confuse maintainers

❌ Don't over-explain in PR:

• Moved detailed timeline analysis to issue
• Kept PR focused on fix validation

❌ Don't add noise:

• No "I think this might work" comments
• No "please review" pings
• No unnecessary updates

10. Professional Communication

In issue:

## Timeline Analysis

I traced this through the codebase history:

1. 2023-05: #27650 set ThrottleInterval to 60s
2. 2023-08: #29078 added kickstart workaround
3. Later: ThrottleInterval reduced to 1s
4. Now: Safe to remove kickstart

[Detailed evidence with links]

In PR:

## Testing Complete ✅

End-to-end testing confirms:
1. Bug reproduced with 2026.3.7
2. Fix validated with PR branch
3. Ready for review

Full logs: [link to issue comment]

The High-Quality PR Formula

1. Deep investigation (post to issue)
2. Minimal, surgical fix
3. Regression test
4. CHANGELOG entry (if project uses it)
5. End-to-end validation
6. Clear PR structure
7. Professional communication
8. Separate concerns (issue vs PR)
9. No internal/irrelevant details
10. Responsive to feedback

PR Lifecycle

Day 1: Investigation
  ├─ Reproduce bug locally
  ├─ Trace through codebase history
  ├─ Identify root cause
  └─ Post detailed analysis to issue

Day 2: Implementation
  ├─ Create minimal fix
  ├─ Add regression test
  ├─ Update CHANGELOG
  └─ Test locally

Day 3: Validation
  ├─ Test with original version (reproduce bug)
  ├─ Test with fixed version (validate fix)
  ├─ Document test results
  └─ Submit PR

Day 4: Refinement
  ├─ Trim PR description (move details to issue)
  ├─ Add context about historical changes
  ├─ Post end-to-end test results
  └─ Mark ready for review

Day 5+: Review cycle
  ├─ Respond to feedback promptly
  ├─ Make requested changes
  └─ Wait for CI and approval

Key Metrics

OpenClaw PR #39763:

• Files changed: 2
• Lines added: ~20 (including tests)
• Lines removed: 3
• PR description: ~50 lines
• Issue investigation: ~200 lines
• Time to first draft: 3 days
• Time to ready: 4 days

What Maintainers Look For

Based on this experience:

1. Does it fix the problem? ✅ Bug reproduced and fixed 2. Is it minimal? ✅ Only changed what's necessary 3. Will it break anything? ✅ Explained why it's safe 4. Can it regress? ✅ Added regression test 5. Is it documented? ✅ CHANGELOG entry 6. Is it tested? ✅ End-to-end validation 7. Is it reviewable? ✅ Clear structure, focused scope

Anti-Patterns We Avoided

1. ❌ Drive-by PR: "Here's a fix, hope it works"

• ✅ We did: Deep investigation, thorough testing

2. ❌ Kitchen sink PR: "Fixed bug + refactored + added features"

• ✅ We did: Minimal, focused fix only

3. ❌ No evidence PR: "Trust me, it works"

• ✅ We did: Reproduced bug, validated fix, posted logs

4. ❌ Wall of text PR: 500-line description

• ✅ We did: Trimmed to ~50 lines, moved details to issue

5. ❌ Ghost PR: Submit and disappear

• ✅ We did: Responsive, iterative refinement

Lessons Learned

Investigation Phase

• Trace through git history to understand context
• Link to all related issues and PRs
• Post detailed analysis to issue, not PR

Implementation Phase

• Make the smallest possible change
• Add regression test
• Follow project conventions exactly

Validation Phase

• Test with original version (prove bug exists)
• Test with fixed version (prove fix works)
• Document both with timestamps and logs

Communication Phase

• Keep PR focused and reviewable
• Move detailed context to issue
• Remove internal/irrelevant details
• Be professional and responsive

Template for Future PRs

## Summary
[1-2 sentences: what this fixes and why]

## Root Cause
[Technical explanation with code references]

## Changes
- [Actual code changes]
- [Tests added]
- [Docs updated]

## Why This Is Safe
[Explain why it won't break anything]

## Testing
[How you validated the fix]

### Test 1: Reproduce Bug
Command: `[command]`
Result: [failure output]

### Test 2: Validate Fix
Command: `[same command]`
Result: [success output]

## Related
- Fixes #[issue]
- Related: #[other issues]

Success Indicators

You know you have a high-quality PR when:

• ✅ Maintainers understand the problem immediately
• ✅ Reviewers can verify the fix easily
• ✅ CI passes on first try
• ✅ No "can you explain..." questions
• ✅ Minimal back-and-forth
• ✅ Quick approval

Final Checklist

Before submitting:

• [ ] Bug reproduced with original version
• [ ] Fix validated with patched version
• [ ] Regression test added
• [ ] CHANGELOG updated (if applicable)
• [ ] PR description is focused (~50 lines)
• [ ] Detailed investigation in issue, not PR
• [ ] No internal tooling mentioned
• [ ] No local paths/secrets in logs
• [ ] All tests pass
• [ ] Follows project conventions

Phase 1 — Pre-PR Discovery

Detailed playbook for the discovery phase: reading CONTRIBUTING.md as a contract, sizing your PR against the project's actual baseline, and writing a scope contract before opening your editor.

1. Reading CONTRIBUTING.md as a contract

Open the file and read it linearly the first time. On the second pass, extract a checklist of the rules that bind your PR. Pay attention to:

1.1 The AI-Assisted Contributions section (if it exists)

Common patterns in projects that have one:

• "You must be able to explain every line." — Implication: no copy-pasted code you can't justify. If a reviewer asks why a line is the way it is and your answer is "Claude wrote it like that", the PR is likely closed.
• "One issue, one PR." — Implication: scope creep is a hard reject. Split before submitting.
• "Open an issue first." — Implication: a same-day issue created right before the PR is better than no issue, but the most respectful path is to file the issue, wait for a maintainer reaction, then PR.
• "Maintainers may close without explanation." — Implication: assume the reviewer is busy and won't engage with a flawed PR. Make it impossible to dismiss.

If the project has such a section, add an "AI-Assisted Disclosure" block to your PR body (see `phase4_pr_description.md`).

1.2 The PR checklist

Most CONTRIBUTING.md files end with a checklist like:

- [ ] pnpm typecheck passes
- [ ] pnpm format:check passes
- [ ] cargo clippy passes (if Rust code changed)
- [ ] Updated i18n files if user-facing text changed

Every unchecked box that applies to your change is a reason for the maintainer to send the PR back. Run the exact commands from CONTRIBUTING.md (not your IDE's variant) and check each box only when you have evidence in front of you.

1.3 Issue templates

If the project uses GitHub issue templates and you're filing an issue first, use the template — don't fill in a freeform issue. Maintainers will close mis-formatted issues faster than they'll close mis-formatted PRs.

1.4 Conventional Commits requirement

Many projects enforce feat(scope): description style via a CI hook. If CONTRIBUTING.md mentions Conventional Commits, audit your local commits before pushing:

git log origin/main..HEAD --format=%s

Each line should match ^(feat|fix|docs|refactor|test|chore|ci|perf|build|revert)(\(.+\))?: .+. Reword commits with git commit --amend or git rebase -i before pushing.

2. Sizing your PR against the project's baseline

A "small PR" is relative to the project. Some projects routinely merge 1000-line refactors; others reject anything over 200 lines without prior discussion.

2.1 Run the baseline query

gh pr list --repo <owner>/<repo> --state merged --limit 10 \
  --json number,title,author,additions,deletions,mergedAt \
  --jq '.[] | "#\(.number) +\(.additions)/-\(.deletions) by \(.author.login): \(.title)"'

Extend to 20 PRs if the recent 10 look unusually skewed (e.g., a single dependabot dominates).

2.2 Interpret the distribution

2.3 How to split a too-large PR

If your work spans multiple logical changes, split along these natural seams:

• Refactor before feature. PR 1: extract or rename interfaces so the feature can land cleanly. PR 2: the feature itself.
• Tests before fix. PR 1: add the failing regression test (skip-marked). PR 2: the fix that unskips it. Easier to review than a single PR with both.
• Backend before frontend. PR 1: API + tests. PR 2: UI consumption.
• Per-AppType / per-module. If your change touches three providers, three PRs are easier to merge than one.

3. The scope contract

Write this paragraph to yourself, before opening your editor:

Goal: <one sentence — what user-visible behavior changes or what specific bug is fixed>
In scope:
  - <bullet — be specific>
  - <bullet>
  - <bullet>
Explicitly out of scope:
  - <bullet — list the temptations you will resist>
  - <bullet>

The "Explicitly out of scope" section is the most useful one. Anticipate the temptations:

• "While I'm in this file I'll fix this unrelated typo." → out of scope.
• "These other 3 functions have the same anti-pattern, I'll fix them too." → out of scope; file a separate issue.
• "I noticed an outdated comment, let me update it." → out of scope.
• "The CI config uses old action versions, let me bump them." → out of scope.

The reason for ruthlessness: every "while I'm here" addition gives a reviewer a new reason to push back, and any one of those reasons can sink the merge.

3.1 Scope contract template

Keep the contract somewhere you'll re-read it — in a SCOPE.md in your worktree, as the body of your draft PR description, or as a sticky note. Re-read it before every commit. If you find yourself making an edit that's not on the "In scope" list, stop and either (a) add it to the list with justification, or (b) revert the edit.

4. Project health quick-check (when choosing what to work on)

If you're picking a project rather than fixing a problem you already have, validate the project is alive before investing time:

gh repo view <owner>/<repo> \
  --json updatedAt,stargazerCount,issues,pullRequests,defaultBranchRef \
  --jq '{
    lastUpdate: .updatedAt,
    stars: .stargazerCount,
    openIssues: .issues.totalCount,
    openPRs: .pullRequests.totalCount,
    defaultBranch: .defaultBranchRef.name
  }'

Red flags:

• lastUpdate more than 6 months ago.
• Many open PRs that haven't been reviewed in months — your PR will sit too.
• Maintainer hostility in recent issue comments.
• No CONTRIBUTING.md at all (some good projects skip it, but it's a yellow flag).

Green flags:

• good first issue label maintained.
• Regular releases (check gh release list).
• Maintainer replies on issues within a week.
• Multiple active maintainers (check gh api repos/<owner>/<repo>/contributors --jq '.[0:5][].login').

See `project_evaluation.md` for the full rubric.

5. Fork hygiene

Always work from upstream main, never from your fork's stale main:

# Inside an existing clone of your fork:
git remote get-url origin   # should be your fork
git remote add upstream https://github.com/<owner>/<repo>.git  # add upstream if missing
git fetch upstream
git switch -c feat/short-name upstream/main

If you've been working on a feature branch for a while and upstream has advanced:

git fetch upstream
git rebase upstream/main
# resolve conflicts, then:
git push origin <branch> --force-with-lease  # never plain --force

See `phase5_post_submission.md` for the full rebase recipe including conflict-resolution patterns.

Phase 2 — Implementation

Detailed playbook for the implementation phase: writing the smallest diff that solves the problem, structuring commits for review, and using the --fixup + autosquash workflow to keep history clean.

1. The minimal-diff principle

A good PR changes only what's necessary. Every unrelated line you touch increases review surface area and gives a reviewer a new place to push back.

1.1 What "necessary" means

A line is necessary if removing your change to it would make your fix incomplete or wrong. Examples:

• ✅ Changing the function body where the bug lives — necessary.
• ✅ Updating a test that asserts the buggy behavior — necessary.
• ✅ Updating the type signature when you added a new parameter — necessary.
• ❌ Reformatting the whole file because your editor saved it — not necessary; revert.
• ❌ Renaming variables for clarity — not necessary unless the rename is the fix.
• ❌ Reorganizing imports — not necessary unless the project's linter explicitly requires it.

1.2 How to enforce the minimal diff

After writing your change, run git diff origin/main..HEAD and read every hunk. For each hunk, ask: "Is this part of the minimal change to ship the feature/fix?" Revert hunks that aren't.

A useful self-check is git diff --stat origin/main..HEAD — if the number of files or lines surprises you, you have unintentional changes.

1.3 What to do with "improvements" you noticed along the way

Two options:

• File an issue. Document the improvement so it isn't lost. Title it clearly: "Refactor: extract X helper" or "Cleanup: rename Y for consistency". Mention it in your PR body as "Noticed during this work, filed as #<issue>".
• Save it for a follow-up PR. Once the current PR merges, open a new branch for the improvement.

2. Commit structure

Each commit should be reviewable on its own — a reviewer should be able to checkout that commit and have a coherent state.

2.1 Conventional Commits

Format: <type>(<scope>): <description> where:

The <scope> is the project's term for the area you touched (e.g., auth, proxy, deeplink). If unsure, look at recent commits with git log --format=%s -20.

The description is imperative present tense, lowercase, no trailing period:

• ✅ feat(deeplink): support extraEnv parameter for provider configuration
• ❌ Added extraEnv support to deeplink (past tense, capitalized)
• ❌ feat: support extra env (missing scope, vague description)

2.2 One logical change per commit

Examples of well-structured commits:

feat(api): add /v1/widgets endpoint
test(api): cover happy-path and validation errors for /v1/widgets
docs(api): document /v1/widgets request/response shape

vs. the anti-pattern:

feat: add widgets endpoint + tests + docs + fix unrelated typo

If you're tempted to make a "various improvements" commit, it's a sign you should split.

2.3 Body text in commit messages

For non-trivial commits, write a body that explains why:

feat(deeplink): support extraEnv parameter for provider configuration

Distributors currently have to ask users to manually flip UI toggles
after importing a provider via deeplink. extraEnv carries a Base64-
encoded JSON object that is merged into settings_config.env, so a
single click can configure the provider end-to-end.

Backward-compatible: extraEnv is optional and serialized with
skip_serializing_if = "Option::is_none". Existing deeplinks parse
unchanged.

The body lives forever in git log; the PR description body lives in GitHub's UI and may be edited or removed.

3. The fixup + autosquash workflow

When a reviewer asks for a change to an existing commit (e.g., "rename this variable" on commit 2 of a 5-commit PR), don't append a "Fix review comments" commit. Instead:

3.1 Make a fixup commit

# Edit the files to address the review comment
git add <changed-files>
git commit --fixup=<sha-of-the-commit-to-amend>

This creates a commit titled fixup! <original commit subject>. Don't push it yet.

3.2 Autosquash

Once you have all your fixup commits ready:

git -c sequence.editor=: rebase -i --autosquash origin/main

The --autosquash flag reorders fixup! commits next to their target and marks them as fixups. The -c sequence.editor=: part is a trick: it sets the rebase sequence editor to : (the no-op shell builtin), which means the rebase plan is accepted as-is without opening an editor. Use this when you trust autosquash to do the right thing automatically.

If you want to inspect or modify the rebase plan first, omit -c sequence.editor=: and your normal $GIT_EDITOR will open.

3.3 Force-push with lease

git push origin <branch> --force-with-lease

--force-with-lease fails if the remote has advanced since your last fetch. This prevents you from clobbering someone else's push (including bot pushes that happen during review). Never use plain `--force` during review — see `phase5_post_submission.md` for the full rationale.

4. Scope creep — the four anti-patterns

These are the most common ways a focused PR turns into a sprawling one.

4.1 "While I'm in here"

You opened provider.rs to fix one function, noticed three other things that could be better, and changed all four. Each of those three other things is an independent decision the reviewer now has to evaluate.

Defense: re-read your scope contract before each commit. Anything not on the "In scope" list goes in a separate PR.

4.2 Rebase-time accidental expansion

While resolving a conflict, you bring in a new feature from upstream that wasn't yours, then "naturally" extend your change to cover it (e.g., upstream added a new enum variant, and you make your code handle it).

Defense: when rebase forces you to integrate with new upstream code, the minimum integration is to leave the new code alone. If extending your feature to cover the new code is unavoidable, declare it in the PR description so the maintainer isn't surprised.

4.3 Tool-assisted "cleanups"

You ran a code-simplifier agent or linter --fix and it touched files unrelated to your change. Those changes are now in your diff.

Defense: review the agent/linter output as carefully as your own. Revert any changes that aren't part of your scope contract. If a cleanup is genuinely valuable, commit it separately, then decide whether it ships in this PR or a follow-up.

In the cc-switch PR #2634 case study, a code-simplifier cleanup got bundled into the fix commit. This made the diff harder to review and almost gave the maintainer a reason to push back. The right move would have been to extract the cleanup into a separate refactor commit.

4.4 Test-coverage expansion

You added one regression test for the bug, then "while I'm at it" added tests for adjacent untested functions. The reviewer now has to evaluate three new tests instead of one.

Defense: the regression test for the bug you fixed is in scope. Tests for adjacent functions go in a test(scope): add coverage for X follow-up PR.

5. Conventional Commit cheat sheet

feat(auth): add OAuth2 PKCE flow
fix(parser): handle empty Base64 input without panicking
refactor(api): extract pagination helper
docs(readme): document new env vars
test(parser): cover Base64 edge cases (empty, whitespace, padded)
chore(deps): bump tokio to 1.46
ci(release): pin actions/checkout to v5
perf(query): cache parsed results across calls
build(makefile): add release-darwin target
revert: feat(auth): add OAuth2 PKCE flow

Scope is optional but recommended. Description is the answer to "If this commit landed, what would change?"

6. Pre-push checklist

Before git push, run mentally:

• [ ] git diff --stat origin/main..HEAD — does the file count and line count match what I intended?
• [ ] git log --format=%s origin/main..HEAD — does every commit message follow Conventional Commits?
• [ ] Are there any console.log, dbg!, debug prints, or commented-out code in the diff?
• [ ] Are there any leftover TODO comments unrelated to this PR?
• [ ] Does each commit pass the project's lint + tests on its own (i.e., bisect-friendly)?

If any answer is no, fix locally before pushing.

Phase 3 — Quality Gates and End-to-End Verification

Detailed playbook for proving your change works before asking a maintainer to trust your word. Covers the automated checks every PR needs, the GUI E2E pattern for desktop apps, and the self-audit step that prevents fabricated test claims.

1. Automated checks (every PR)

Run the project's full lint + test suite locally. The exact commands come from CONTRIBUTING.md. Examples from real projects:

# Node / TypeScript projects
pnpm typecheck
pnpm format:check
pnpm lint
pnpm test:unit

# Rust projects
cargo fmt --check
cargo clippy --all-targets -- -D warnings
cargo test

# Python projects
ruff check .
ruff format --check
pytest

# Go projects
gofmt -l .
go vet ./...
go test ./...

Run each command individually and capture the output. If a command takes more than ~30 seconds, save the output to a file so you can paste it into the PR body later:

pnpm test:unit 2>&1 | tee /tmp/test-unit.log

1.1 If a check fails

Do not push. Fix the failure locally. Common categories:

• Format failure: run the formatter (pnpm format, cargo fmt, ruff format). Re-run --check.
• Lint failure: fix the lint or, if the project allows, add a documented ignore at the call site. Avoid global ignore unless the project's own config does it.
• Type failure: fix the type. Avoid // @ts-ignore, // nolint, // type: ignore unless the project uses them elsewhere for the same pattern.
• Test failure: if the failing test is one you didn't touch, suspect your change broke something unrelated. Run git stash && <test command> to confirm whether main is also broken.

1.2 If CI runs additional checks the project's CONTRIBUTING.md doesn't list

Inspect .github/workflows/ for the project's actual CI matrix. Some projects only document the headline checks in CONTRIBUTING.md but enforce additional ones in CI (e.g., integration tests, Docker builds, security scans). Running these locally is optional but increases first-push success.

2. The isolated-home pattern for desktop apps

Desktop apps (Tauri, Electron, Cocoa, Qt, GTK) almost always read configuration and data from a fixed location like ~/.appname/ or ~/Library/Application Support/com.app.id/. Running the dev binary will read your real user data.

The pattern:

1. Find the project's test hook that overrides the data directory. 2. Point the test hook at `/tmp/` before launching the dev binary. 3. Trigger the feature through whatever real interface a user would use. 4. Verify by reading the persisted state directly (SQLite, JSON files), not just by visual inspection.

2.1 Finding the test hook

Common naming patterns:

• <APPNAME>_TEST_HOME, <APPNAME>_DATA_DIR, <APPNAME>_CONFIG_DIR
• XDG_DATA_HOME, XDG_CONFIG_HOME (Linux-style, sometimes honored on macOS too)
• A config file flag (--data-dir=, --profile=)

Grep for the candidate names in the project's source:

rg -i 'TEST_HOME|TEST_DIR|test_home|test_dir|DATA_DIR' --type rust --type ts
rg 'env::var\(' src-tauri/  # Rust: env reads
rg 'process\.env\.' src/    # Node: env reads

If you find a function like get_home_dir() that reads an environment variable as an override, that's your hook.

If the project has no test hook, the safest path is:

• Back up your real data first (cp -a ~/.appname ~/.appname.bak).
• Open an issue suggesting adding a test hook (it costs the maintainer ~5 lines).
• Use a pre-built isolated VM/container if the project provides one (less common).

Never attempt to "just be careful" with your real data. You will eventually clobber it.

2.2 Real example: cc-switch

cc-switch hardcodes its data directory but provides CC_SWITCH_TEST_HOME:

// src-tauri/src/config.rs (paraphrased)
pub fn get_home_dir() -> PathBuf {
    if let Ok(home) = std::env::var("CC_SWITCH_TEST_HOME") {
        let trimmed = home.trim();
        if !trimmed.is_empty() {
            return PathBuf::from(trimmed);
        }
    }
    dirs::home_dir().unwrap_or_default()
}

Usage:

mkdir -p /tmp/cc-switch-e2e/.cc-switch
CC_SWITCH_TEST_HOME=/tmp/cc-switch-e2e pnpm tauri dev

The dev binary now reads/writes /tmp/cc-switch-e2e/.cc-switch/cc-switch.db, never touching ~/.cc-switch/.

Confirm isolation worked by reading the dev log on startup:

[INFO] MCP table empty, importing from live configurations...
[INFO] Prompts table empty, importing from live configurations...
[INFO] No Claude MCP servers found to import

These "empty" / "no servers found" messages indicate a fresh database. If you see "imported 47 sessions from existing data", isolation failed — kill the binary and investigate.

3. Triggering features without polluting the system

For URL-scheme features (deeplinks), the temptation is to type open ccswitch://... in the terminal. Do not — macOS LaunchServices routes the URL to the installed .app, not your dev binary. You'll modify your real user data.

3.1 Tauri 2 single-instance forward

Tauri 2's single_instance plugin lets you re-launch the binary with the URL as argv[1]. The running dev instance receives the URL through its single_instance callback:

CC_SWITCH_TEST_HOME=/tmp/cc-switch-e2e \
  ./src-tauri/target/debug/cc-switch "ccswitch://v1/import?resource=provider&app=claude&..."

This bypasses LaunchServices entirely and routes the URL to your specific dev binary instance.

Watch the dev log for confirmation:

[INFO] === Single Instance Callback Triggered ===
[INFO] ✓ Deep link URL detected from single_instance args: <url>
[INFO] ✓ Successfully parsed deep link: <details>

3.2 Generalizing the pattern to other stacks

If the project doesn't have a test-friendly trigger mechanism, file an issue suggesting one (or contribute it as your first PR before the feature PR).

4. Direct state verification

Visual inspection of the GUI is necessary but not sufficient. Read the persisted state directly:

4.1 SQLite

sqlite3 /tmp/<isolated-data-dir>/<db-file> ".tables"
sqlite3 /tmp/<isolated-data-dir>/<db-file> "SELECT * FROM <table> WHERE name='<test-record>'"

For complex blob columns (JSON in SQLite), pipe through python3 -c 'import json,sys; print(json.dumps(json.loads(sys.stdin.read()), indent=2))'.

4.2 JSON / TOML / plain files

cat /tmp/<isolated-data-dir>/settings.json | jq .

4.3 Verification matrix

For non-trivial changes, build a table of expected vs. actual for every behavior your change affects. Example from cc-switch PR #2634:

Paste this matrix into the PR description. It's denser and more verifiable than prose.

5. Capturing GUI screenshots

For the PR's "Screenshots" section, capture only what's relevant to your change. Don't paste full-screen screenshots — they contain noise.

5.1 macOS

# Full-screen capture
screencapture -x /tmp/screenshot.png

# Single window (interactive selection)
screencapture -W /tmp/screenshot.png

# Specific area (interactive selection)
screencapture -s /tmp/screenshot.png

# Specific window ID (no interaction)
screencapture -l <window-id> /tmp/screenshot.png

To get the window ID for your dev app:

osascript -e 'tell application "System Events" to get id of front window of (first process whose name is "<app-name>")'

5.2 Bring the app to the front before capturing

osascript -e 'tell application "System Events" to set frontmost of (first process whose name is "<app-name>") to true'

In some setups osascript focus calls are unreliable (the terminal can steal focus back). A more reliable trigger is the app's own focus call — e.g., for cc-switch, re-running the binary triggers the single-instance callback which calls window.set_focus() internally.

5.3 Crop after capturing

Use Python + Pillow to crop noise out of full-screen captures:

from PIL import Image
img = Image.open('/tmp/screenshot.png')
# Detect content bounds by finding white-ish pixels (the app window)
crop = img.crop((<left>, <top>, <right>, <bottom>))
crop.save('/tmp/screenshot_cropped.png', optimize=True)

Aim for tight crops that show one piece of UI clearly. A reviewer should be able to understand the screenshot in 2 seconds.

6. Self-audit: did you actually do everything you're about to claim?

Before writing the PR description, list every claim you intend to make:

Claims I plan to make in the PR body:
- "All unit tests pass" → evidence: `pnpm test:unit` output in /tmp/test-unit.log
- "Lint clean" → evidence: `cargo clippy --all-targets` exited 0
- "Tested end-to-end with isolated home" → evidence: dev log + SQLite dump in /tmp/cc-switch-e2e/
- "No production data was touched" → evidence: I used CC_SWITCH_TEST_HOME the entire time
- "Screenshot 1: import dialog" → evidence: /tmp/e2e/screenshot_1_import_dialog.png
- "Screenshot 2: env block" → evidence: /tmp/e2e/screenshot_2_env_block.png

For each claim, can you produce the evidence in 5 seconds? If not, the claim is at risk of being fabrication. Either run the test now or remove the claim from the PR body.

Why this matters: the single fastest way to lose maintainer trust is to claim something you didn't actually do, and have the maintainer try to reproduce it. Maintainers have long memories about contributors who waste their time.

This is also why screenshots are valuable — they're evidence the GUI behavior you describe actually exists. Prose alone is not evidence.

Phase 4 — Writing the PR Description

Detailed playbook for the PR description: structure, the test-coverage-matrix pattern, the AI-Assisted Disclosure block, and screenshot embedding without polluting the repo.

1. What the PR description is for

The PR description has three jobs, in order of importance:

1. Let the maintainer decide in 30 seconds whether this is worth merging. 2. Give reviewers everything they need to verify without DM'ing you. 3. Create a written record that survives team turnover.

A PR description optimizes for the reviewer's time, not yours. Write longer if the change is non-trivial — but every paragraph must earn its place.

2. Body skeleton

## Summary / 概述
<2 sentences max — what changed, why it matters>

## What / 变更内容
<bulleted list of commits OR files, with their purpose>

## Why / 动机
<the problem this solves; if no prior issue, justify the change in 1 paragraph>

## Test Plan / 测试计划
<exact commands a maintainer can run; coverage matrix for non-trivial changes>

## How to verify locally / 如何本地验证
<copy-pasteable commands that produce the evidence>

## Backward Compatibility / 向后兼容
<state explicitly; don't make the maintainer infer>

## Security Considerations
<only if the change touches auth, untrusted input, or shared state>

## Screenshots / 截图
<placeholder for the user to drag images into the GitHub web UI>

## Related Issue
<Fixes #N, or explain why no issue exists>

## Checklist
<copy of the project's PR template checklist, with real evidence per box>

## AI-Assisted Disclosure
<if CONTRIBUTING.md mentions AI contributions — see section 5>

Bilingual headings are optional but appreciated by international projects with mixed-language maintainers. Use them if the project's own commit messages or issue templates are bilingual.

3. The Summary section

Two sentences. The first describes what changed; the second describes why it matters or what enables.

Good:

Adds an optional extraEnv Base64-encoded JSON parameter to provider deeplinks. This lets distributors ship pre-configured providers in a single click instead of asking users to manually flip UI toggles after import.

Bad:

This PR adds a new feature for deeplinks. It's been a long process but I finally figured it out. Hopefully this is useful.

The first version answers "what" and "why" with concrete nouns. The second answers neither.

4. The Test Plan section

4.1 Test coverage matrix

When your PR adds more than 2-3 tests, present them as a table. The maintainer can scan the table once instead of reading test code.

| Layer | Test | What it proves |
|---|---|---|
| URL parsing | `test_parse_provider_with_extra_env` | `extraEnv` query param extracted to `Option<String>` |
| Build (happy path) | `test_build_claude_settings_with_extra_env` | Claude `env` block merged correctly |
| Build (backward compat) | `test_extra_env_does_not_break_without_value` | Absent `extraEnv` → unchanged behavior |
| Build (error tolerance) | `test_extra_env_ignores_invalid_base64` | Garbage Base64 → logged, skipped, import continues |
| Security | `test_extra_env_stringifies_scalars_and_skips_invalid_values` | Bools/numbers stringified; null/array/object dropped |
| Integration | `deeplink_import_claude_provider_persists_to_db` | Full DB round-trip with `extraEnv` |

4.2 Verified-locally checklist with real output

Include the exact commands you ran and a short proof:

### Verified on this branch tip (\`<commit-sha>\`):

\```bash
$ cd src-tauri && cargo test --lib deeplink
test result: ok. 40 passed; 0 failed; 0 ignored

$ cargo test --test deeplink_import
test result: ok. 5 passed; 0 failed; 0 ignored

$ pnpm test:unit
Test Files  36 passed (36)
     Tests  223 passed (223)

$ cargo clippy --all-targets   # clean
$ cargo fmt --check            # clean
$ pnpm typecheck               # clean
$ pnpm format:check            # clean
\```

This is short, copy-pasteable, and a reviewer can reproduce it in one minute.

5. AI-Assisted Disclosure block

If CONTRIBUTING.md has an AI-assisted contribution clause (or the project's maintainer has commented skeptically on past AI-assisted PRs), add this block at the bottom of your description. Be specific, not generic.

## AI-Assisted Disclosure

Per CONTRIBUTING.md §<section-number>:

1. **I have read every line.** Happy to walk through any function or design choice on request. Specifically: <one example of a non-obvious choice you can defend>.
2. **Tested locally.** <list the actual commands you ran and the actual results — don't be vague>. Cannot personally verify <platform you don't have>; no platform-specific APIs are touched.
3. **Single-topic.** This PR is scoped to <one sentence>. The <name of any tool-assisted cleanup>'s changes are confined to <files> and total <N> lines; if you'd prefer them split out I can do that.
4. **<Issue status>.** <Either: "Fixes #N" / "No prior issue; happy to open one retroactively if you'd prefer that paper trail before merging."> 
5. **AI tools used.** Claude Code for drafting; <list any others, e.g., Codex's automated review, GitHub Copilot>. <Note any specific findings AI tools drove — e.g., "Codex's P1+P2 review directly drove the hardening work in commit 2.">. Final review and decisions are mine.

The disclosure does not excuse poor work — but missing it on a project that requires it is an instant trust hit. Treat it as a hard requirement when CONTRIBUTING.md mentions AI contributions.

6. Screenshot embedding without polluting the repo

6.1 The problem

gh CLI does not support image attachments to PR descriptions. GitHub's upload endpoint at uploads.github.com requires browser session cookies + CSRF tokens, not PAT tokens. Community tools (gh-attach, gh-pic) reverse-engineer the undocumented API and break intermittently.

6.2 The cleanest approach: placeholders + user drag-drop

1. In your local PR draft (e.g., /tmp/pr_body.md), leave clearly-named placeholders:

   ### Screenshot 1: import dialog
   
   [E2E_SCREENSHOT_1_PLACEHOLDER — drag screenshot_1_import_dialog.png here]
   
   ### Screenshot 2: edit provider showing env block
   
   [E2E_SCREENSHOT_2_PLACEHOLDER — drag screenshot_2_env_block.png here]

2. Push the description: gh pr edit <pr> --body-file /tmp/pr_body.md.

3. Tell the user (or do yourself): open the PR in browser, click Edit (pencil icon), find each placeholder, delete it, drag the corresponding image file from Finder into the markdown area. GitHub uploads to user-images.githubusercontent.com and replaces the placeholder with ![](url).

4. Save.

Zero repo pollution. Images live on GitHub's CDN.

6.3 Fallback: orphan branch on your fork

If you can't have a human do the drag-drop step (e.g., automation pipeline):

# Create an orphan branch holding only screenshots
git worktree add -b assets/pr-<N>-screenshots /tmp/assets-worktree
cd /tmp/assets-worktree
git rm -rf .  # remove everything from the orphan branch
cp /tmp/screenshot_*.png .
git add .
git commit -m "screenshots for PR #<N>"
git push fork assets/pr-<N>-screenshots
cd -
git worktree remove /tmp/assets-worktree

Reference in PR body:

![](https://raw.githubusercontent.com/<your-fork-user>/<repo>/assets/pr-<N>-screenshots/screenshot_1_import_dialog.png)

This pollutes your fork (extra branch) but not the PR diff (the orphan branch is independent).

6.4 Last resort: third-party image host

Imgur, Cloudinary, etc. Be aware:

• Privacy of the image is unclear (some hosts make uploads public regardless of "private" flags).
• Persistence is unclear (free hosts may garbage-collect).
• Don't use for anything sensitive (internal URLs, screenshots of UI showing user data).

7. Length guidance

There is no fixed maximum. The right length depends on the change's complexity. Rough guidance:

If you're past 700 lines, add a ## Table of Contents at the top so reviewers can jump.

If you're under 50 lines, double-check you've included all the evidence — short PR bodies often miss the Test Plan section.

8. Checklist box evidence

The project's PR template likely has a checklist. Don't just tick the boxes — provide one-line evidence for each:

## Checklist
- [x] \`pnpm typecheck\` passes — verified (`tsc --noEmit`, clean)
- [x] \`pnpm format:check\` passes — verified (Prettier, clean)
- [x] \`cargo clippy --all-targets\` passes — verified (no warnings)
- [x] \`cargo fmt --check\` passes — verified
- [x] \`cargo test\` passes — verified (40 lib + 5 integration, 0 failures)
- [x] No user-facing strings added; no i18n updates needed
- [x] Conventional Commits format (\`feat(deeplink): …\`, \`fix(deeplink): …\`)

The "— verified (...)" suffix turns a checkbox into a verifiable claim. A reviewer can spot-check any one.

Phase 5 — Post-Submission

Detailed playbook for what happens after you push: responding to automated bot reviews, resolving conflicts when upstream advances, force-pushing safely, and filtering counter-review noise.

1. Responding to bot reviews (Codex, Claude bot, CodeRabbit, etc.)

Modern projects use AI bots for first-pass review. Their comments appear as review comments on specific lines, not as PR-level comments. Each finding gets its own thread, and your reply should appear under the finding (not as a separate PR-level comment), so future reviewers see the resolution next to the original concern.

1.1 Find the finding's comment ID

A review comment's URL ends in #discussion_rXXXXXXXX. The number is the comment ID. Alternatively:

gh api repos/<owner>/<repo>/pulls/<pr>/comments \
  --jq '.[] | select(.user.login == "chatgpt-codex-connector[bot]") | {id, body: .body[0:80], path, line}'

This lists all bot review comments with their IDs and the line they target.

1.2 Reply to a specific finding

gh api repos/<owner>/<repo>/pulls/<pr>/comments \
  -X POST \
  -F in_reply_to=<comment-id> \
  -f body="Addressed in commit \`<sha>\`: <specific change>. Regression locked in by \`<test-name>\`. Thanks for the catch!"

The reply appears threaded under the original finding. The maintainer sees the resolution without searching.

1.3 Reply template

Addressed in commit \`<sha>\`:

- <function or check>: <one-sentence description of what changed>
- Regression locked in by \`<test-name>\`

Thanks for the catch!

Be specific. "Fixed in latest" is not enough — the reviewer should be able to verify your claim in 30 seconds by checking the named commit/test.

1.4 When the bot is wrong

If the bot's finding is wrong or doesn't apply, still reply — don't ignore. Silence reads as "didn't notice". Acceptable replies:

Not a real issue here — X is already validated upstream by Y. Leaving as-is.

The bot is flagging a false positive: this path is only reached when Z is true, and we check that two lines up.

Considered this but decided the fix would be more risky than the issue. Open to changing if you disagree.

A human maintainer reviewing later will see your reasoning and either accept it or push back.

2. Rebase when upstream advances

When upstream main lands new commits while your PR is open, GitHub may show "This branch is N commits behind". Most of the time you don't need to do anything — GitHub merges your PR onto current main at merge time. But if there's a real conflict, you'll see "This branch has conflicts that must be resolved" and you'll need to rebase.

2.1 Standard rebase

git fetch origin
git rebase origin/main

If conflicts occur:

CONFLICT (content): Merge conflict in <file>

Open the file, resolve the conflict markers manually, then:

git add <resolved-files>
git -c sequence.editor=: rebase --continue

(The -c sequence.editor=: part skips the commit message editor for the resolved commit, accepting the existing message.)

If you need to abort and start over:

git rebase --abort

2.2 Force-push with lease

After a successful rebase, your local branch has a different history from the remote. You must force-push:

git push origin <branch> --force-with-lease

Why `--force-with-lease` and not `--force`:

• --force overwrites the remote branch unconditionally. If anyone else (or any bot) pushed to your branch since your last fetch, their commits are lost without warning.
• --force-with-lease aborts if the remote tip has moved since your last fetch. You'll see "stale info" error and know to fetch + investigate.

In a review context, bot reviews (Codex, Claude bot) sometimes push commits to your branch or post commits as comments — you don't always notice. --force-with-lease protects against destroying those silently.

If you legitimately need to overwrite even what the bot pushed:

git fetch origin <branch>
# Inspect the remote tip with: git log origin/<branch> --oneline -5
# Decide if it's safe to discard, then:
git push origin <branch> --force-with-lease=<branch>:<expected-remote-sha>

This is the precise form: "Only force-push if the remote tip is exactly <expected-remote-sha>."

2.3 Handling conflict in code you didn't write

If upstream's new code conflicts with yours, you have to integrate. The minimum integration is to leave the new code as-is and put yours alongside.

Anti-pattern: extending your feature to "naturally" cover the new upstream code. That's scope creep at rebase time — see `phase2_implementation.md` §4.2.

If extension is unavoidable (e.g., upstream added an enum variant your match statement must handle), declare it in the PR description:

## Note on rebase

Upstream landed <feature> in [#NNNN](link) during this PR's review. The rebase
required extending <my-feature> to cover the new variant. The diff for that
integration is in commit <sha>; happy to split into a separate PR if you'd prefer.

The maintainer is much more receptive to a declared scope expansion than a discovered one.

3. Force-push during active review

If your PR is in active review (a maintainer or bot has left comments), avoid force-pushing if possible. Reasons:

• Some review comments are anchored to specific commits; force-push can orphan them.
• The reviewer's mental model is "I last reviewed at commit X" — if you replace history, they have to re-orient.
• It looks evasive ("did the contributor delete my comment thread?").

Prefer appending fixup commits during active review:

# After review comment, edit files
git add <changed>
git commit -m "fix review: handle empty case"  # or use --fixup= for autosquash later
git push origin <branch>  # no force needed

Once review is complete and the maintainer is ready to merge, you can rebase + autosquash to clean up the history if the project's CI requires it (some projects squash-merge anyway).

4. Filtering counter-review output

If you run a counter-review agent (or get flooded with 20+ bot findings), don't paste them all into the PR. Filter ruthlessly using three lenses:

For each finding that survives the filter, address it (in code or with a reply explaining why you accepted vs. declined). Discard the rest silently — they're noise, not signal.

4.1 What "noise" looks like in practice

• Type-system over-defense: "Wrap this unwrap() in a match even though the input is statically known to be Some."
• Premature optimization: "Cache this lookup that runs once per program startup."
• Style preferences disguised as bugs: "Reorder these arguments alphabetically."
• Spec-mismatch where the spec is yours: "This doesn't match the standard X" — when the project deliberately diverges from X.

A good counter-review agent surfaces things you missed. A mediocre one floods you with low-value findings. Don't reward the latter by treating every finding as actionable.

5. Responding to a maintainer's substantive review

Different from bot review: a human maintainer's comment is a signal of engagement. Reply quickly (within 24h is ideal) and constructively.

5.1 Accepting a change request

Good point — updated in <sha>: <one-line summary of what changed>.

5.2 Explaining a deliberate choice

I chose this approach because <reason 1>, <reason 2>. The tradeoff is <X>, but I weighed it against <Y> and went this way. Open to switching if you'd prefer <alternative>.

5.3 Requesting clarification

Thanks for the feedback — could you clarify what you mean by "<quote>"? I want to make sure I address the right concern.

5.4 Disagreeing respectfully

I see your point about <X>. The current approach was chosen because <reason>, but I understand the concern.

Would a middle-ground like <alternative> work for you? It addresses <maintainer's concern> while keeping <my benefit>.

The pattern: acknowledge → reason → offer compromise. Never escalate to "you're wrong".

See `communication_templates.md` for more response templates.

6. After merge

Once your PR merges, send a brief thank-you and close the loop:

Thanks for the review and merge! Learned <one specific thing> from the feedback — will apply that in future contributions.

This:

• Closes the conversation politely.
• Signals you read and absorbed the feedback (not just complied).
• Builds a reputation for future PRs.

Then:

• Delete your local feature branch: git branch -d <branch>.
• Delete your fork's feature branch (GitHub UI offers a button after merge).
• Update your fork's main: git fetch upstream && git switch main && git merge upstream/main && git push origin main.

You're ready for the next PR.

PR Quality Checklist

Complete checklist for creating high-quality pull requests.

PR Quality Checklist

Complete checklist for creating high-quality pull requests based on successful contributions to major open-source projects.

Investigation Phase (Before Coding)

• [ ] Read CONTRIBUTING.md thoroughly
• [ ] Check for existing PRs addressing same issue
• [ ] Comment on issue to express interest
• [ ] Reproduce bug with original version
• [ ] Trace git history for context (git log --all --grep="keyword")
• [ ] Identify root cause with code references
• [ ] Post detailed investigation to issue (not PR)
• [ ] Link all related issues/PRs

Investigation Template (Post to Issue)

## Investigation

I traced this through the codebase history:

1. [Date]: #[PR] introduced [feature]
2. [Date]: #[PR] added [workaround] because [reason]
3. [Date]: #[PR] changed [parameter]
4. Now: Safe to [fix] because [explanation]

[Detailed evidence with code references]

Before Starting

Environment Setup

• [ ] Fork repository
• [ ] Clone to local machine
• [ ] Set up development environment
• [ ] Run existing tests (ensure they pass)
• [ ] Create feature branch with descriptive name

# Branch naming conventions
feature/add-yaml-support
fix/resolve-connection-timeout
docs/update-installation-guide
refactor/extract-validation-logic

During Development

• [ ] Make minimal, focused changes (only what's necessary)
• [ ] Add regression test to prevent future breakage
• [ ] Update CHANGELOG if project uses it
• [ ] Follow project's commit message format
• [ ] Run linter/formatter before committing
• [ ] Don't refactor unrelated code
• [ ] Don't add "improvements" beyond the fix

What to Change

✅ Do change:

• Code directly related to the fix
• Tests for the fix
• CHANGELOG entry
• Relevant documentation

❌ Don't change:

• Surrounding code (refactoring)
• Unrelated files
• Code style of existing code
• Add features beyond the fix

Before Submitting

Code Quality

• [ ] All tests pass
• [ ] No linter warnings
• [ ] Code follows project style
• [ ] No unnecessary changes (whitespace, imports)
• [ ] Comments explain "why", not "what"

Evidence Loop

• [ ] Test with original version and capture failure output
• [ ] Apply the fix
• [ ] Test with fixed version and capture success output
• [ ] Document both tests with timestamps, exit codes, PIDs
• [ ] Compare baseline vs fixed behavior

Testing Commands

# Test 1: Reproduce bug with original version
npm install -g package@original-version
[command that triggers bug]
# Capture: error messages, exit codes, timestamps

# Test 2: Validate fix with patched version
npm install -g package@fixed-version
[same command]
# Capture: success output, normal exit codes

Redaction Gate

• [ ] Remove local absolute paths (for example, /Users/...) from logs and screenshots
• [ ] Remove secrets/tokens/API keys from logs and screenshots
• [ ] Remove internal URLs/hostnames from logs and screenshots
• [ ] Recheck every pasted block before submitting

Documentation

• [ ] README updated (if applicable)
• [ ] API docs updated (if applicable)
• [ ] Inline comments added for complex logic
• [ ] CHANGELOG updated (if required)

PR Description

• [ ] Clear, descriptive title (conventional commit format)
• [ ] Focused description (~50 lines, not >100)
• [ ] Summary (1-2 sentences)
• [ ] Root cause (technical, with code refs)
• [ ] Changes (bullet list)
• [ ] Why it's safe
• [ ] Testing validation
• [ ] Related issues linked
• [ ] No detailed timeline (move to issue)
• [ ] No internal tooling mentions
• [ ] No speculation or uncertainty

PR Description Length Guide

✅ Good: ~50 lines

• Summary: 2 lines
• Root cause: 5 lines
• Changes: 5 lines
• Why safe: 5 lines
• Testing: 20 lines
• Related: 3 lines

❌ Too long: >100 lines

• Move detailed investigation to issue
• Move timeline analysis to issue
• Keep PR focused on the fix

PR Title Format

<type>(<scope>): <description>

Examples:
feat(api): add support for batch requests
fix(auth): resolve token refresh race condition
docs(readme): add troubleshooting section
refactor(utils): simplify date parsing logic
test(api): add integration tests for search endpoint
chore(deps): update lodash to 4.17.21

PR Description Template

````markdown

Summary

[1-2 sentences: what this fixes and why]

Root Cause

[Technical explanation with code references]

Changes

• [Actual code changes]
• [Tests added]
• [Docs updated]

Why This Is Safe

[Explain why it won't break anything]

Testing

Test 1: Reproduce Bug (Original Version)

Command: [command] Result:

[failure output with timestamps, exit codes]

Test 2: Validate Fix (Patched Version)

Command: [same command] Result:

[success output with timestamps, exit codes]

Related

• Fixes #[issue]
• Related: #[other issues/PRs]

````

What NOT to include:

• ❌ Detailed timeline analysis (put in issue)
• ❌ Historical context (put in issue)
• ❌ Internal tooling mentions
• ❌ Speculation or uncertainty
• ❌ Walls of text (>100 lines)

Comparison Table Template

| Case | Command / Scenario | Result | Evidence |
|------|--------------------|--------|----------|
| Baseline | `[same command]` | Fail | [raw output block] |
| Fixed | `[same command]` | Pass | [raw output block] |
| Reference | [spec, issue, or main behavior] | Expected | [link or note] |

After Submitting

• [ ] Monitor for CI results
• [ ] Respond to review comments within 24 hours
• [ ] Make requested changes quickly
• [ ] Thank reviewers for their time
• [ ] Don't force push after review starts (unless asked)
• [ ] Add new commits during review (don't amend)
• [ ] Explain what changed in follow-up comments
• [ ] Re-request review when ready

Separation of Concerns

Issue Comments (Detailed Investigation)

• Timeline analysis
• Historical context
• Related PRs/issues
• Root cause deep dive
• 100-300 lines OK

PR Description (Focused on Fix)

• Summary (1-2 sentences)
• Root cause (technical)
• Changes (bullet list)
• Testing validation
• ~50 lines total

Separate Test Comment (End-to-End Validation)

• Test with original version
• Test with fixed version
• Full logs with timestamps

Review Response Etiquette

Good Responses

"Good point! I've updated the implementation to..."

"Thanks for catching that. Fixed in commit abc123."

"I see what you mean. I chose this approach because...
Would you prefer if I changed it to...?"

Avoid

"That's just your opinion."

"It works on my machine."

"This is how I always do it."

Common Rejection Reasons

1. Too large - Break into smaller PRs 2. Unrelated changes - Remove scope creep 3. Missing tests - Add test coverage 4. Style violations - Run formatter 5. No issue link - Create or link issue first 6. Conflicts - Rebase on latest main

Project Evaluation Guide

How to evaluate open-source projects before contributing.

Prerequisites

• Install GitHub CLI and verify availability: gh --version
• Authenticate before running commands: gh auth status || gh auth login

Quick Health Check

# Check recent activity
gh repo view owner/repo \
  --json updatedAt,stargazerCount,issues \
  --jq '{updatedAt, stargazers: .stargazerCount, openIssues: .issues.totalCount}'

# Check PR response time
gh pr list --repo owner/repo --state merged --limit 10

# Check issue activity
gh issue list --repo owner/repo --state=open --limit 20

Evaluation Criteria

1. Activity Level

2. Community Health

3. Maintainer Engagement

4. Documentation Quality

Scoring System

Rate each category 1-5:

Activity Level:      _/5
Community Health:    _/5
Maintainer Engage:   _/5
Documentation:       _/5
----------------------------
Total:               _/20

Interpretation:

• 16-20: Excellent choice
• 12-15: Good, proceed with caution
• 8-11: Consider carefully
• < 8: Avoid or expect delays

Red Flags

Immediate Disqualifiers

• No commits in 1+ year
• Maintainer explicitly stepped away
• Project archived
• License issues

Warning Signs

• Many open PRs without review
• Hostile responses to contributors
• No clear contribution path
• Overly complex setup

Green Flags

Strong Indicators

• "good first issue" labels maintained
• Active Discord/Slack community
• Regular release schedule
• Responsive maintainers
• Clear roadmap

Bonus Points

• Funded/sponsored project
• Multiple active maintainers
• Good test coverage
• CI/CD pipeline

Research Checklist

Project Evaluation:
- [ ] Check GitHub Insights
- [ ] Read recent issues
- [ ] Review merged PRs
- [ ] Check contributor guide
- [ ] Look for "good first issue"
- [ ] Assess community tone
- [ ] Verify active maintenance
- [ ] Confirm compatible license

Finding Projects

By Interest

# Find by topic
gh search repos "topic:cli" --sort=stars

# Find by language
gh search repos "language:python" --sort=stars

# Find with good first issues
gh search issues "good first issue" --language=rust --state=open

By Need

• Tools you use daily
• Libraries in your projects
• Frameworks you're learning
• Problems you've encountered

Curated Lists

• awesome-for-beginners
• first-timers-only
• up-for-grabs.net
• goodfirstissue.dev