Strategic guide for becoming an effective GitHub contributor. Covers opportunity discovery, project selection, high-quality PR creation, and reputation building...
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).
Use this skill when all of these are true:
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).
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.
CONTRIBUTING.md is not style advice. Treat every numbered rule as a precondition for merge. Pay special attention to:
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.
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.
A scope contract is a single paragraph you write to yourself before opening your editor:
Goal:
. 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.
main immediately after fetching upstreamgit 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.
Resist any change that is not directly required by your scope contract. In particular:
If a follow-up improvement is genuinely valuable, file a separate issue or open a separate PR after this one is merged.
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.
Maintainers' trust is built by evidence, not by claims. The point of this phase is to produce evidence you can paste into the PR.
Read CONTRIBUTING.md and the current CI workflow for the required commands and platform matrix. Reuse successful checks on the exact same content when their inputs remain applicable; otherwise run the missing or affected checks. Typical examples (use what your project specifies):
pnpm typecheck && pnpm format:check && pnpm test:unit
cargo fmt --check && cargo clippy --all-targets && cargo test
For a failure, compare the same command against the immutable current base in an isolated worktree before attributing it to the PR. Preserve failed results and distinguish a regression from an existing defect or environment blockage; never stop another session's service to make a test pass. Complete the repository's required checks before claiming merge readiness.
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:
XXX_TEST_HOME, XXX_DATA_DIR, or a config flag in config.rs / paths.go)./tmp/<app-name>-e2e/ before launching.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.
For changes with downstream behavior, use
behavioral impact review
to select preservation tests before accepting a local pass, and again after a
review-led repair. Resolve that reference through the installed github-review-pr
Skill when contributing outside this marketplace; if it is unavailable, require
actual consumer checks against the original result/preservation contract and
report the missing reference. This does not invoke an independent review or grant
maintainer authority.
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.
When the user explicitly asks for an independent maintainer-style evaluation of their
own contribution, invoke github-review-pr:github-review-pr for its read-only review
method. Give fresh-context reviewers the original issue/review discussion and immutable
head/base objects. Keep the requester in the author role: this produces a recommendation,
not a formal approval or permission to merge. Continue repairs through this contributor
workflow only within existing authorization. Ordinary self-audits do not trigger it.
Local tests passing is not the finish line. Before you call the PR merge-ready, run the push-time checklist:
gh repo view <owner>/<repo> --json visibility,isPrivate,defaultBranchRef
--no-verify.git config --global --get-regexp url for stale URL rewrites.git push succeeding does not mean GitHub can merge:gh pr view <pr-number> --repo <owner>/<repo> --json mergeable,mergeStateStatus
Full details (URL rewrites, PII-hook false positives, --force-with-lease caveats) are in references/push_time_gotchas.md.
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.
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>
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 |
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:
[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.assets-pr-N-screenshots), commit images, reference them via raw.githubusercontent.com. Pollutes your fork but not the PR diff.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.
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.
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.
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:
| Filter | Discard if |
|---|---|
| Probability | "Could this actually happen in this codebase?" โ No |
| Cost | "Would fixing it cost more than the risk?" โ Yes |
| Scenario | "Is this scenario already prevented upstream?" โ Yes |
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.
| File | Use for |
|---|---|
references/phase1_discovery.md |
CONTRIBUTING.md parsing, PR size baseline rubric, scope-contract templates |
references/phase2_implementation.md |
Fixup commit + autosquash workflow, scope-discipline anti-patterns |
references/phase3_quality_gates_and_e2e.md |
Isolated-home pattern, single-instance forward, SQLite verification, screencapture + window focus |
references/phase4_pr_description.md |
Body skeleton, test-coverage-matrix, AI disclosure templates, screenshot placeholder pattern |
references/phase5_post_submission.md |
gh api in_reply_to recipe, --force-with-lease semantics, counter-review filtering |
references/push_time_gotchas.md |
Git remote URL rewrites, PII-hook false positives, mergeability verification, force-push caveats |
references/case_study_cc-switch_pr_2634.md |
Full real-world walkthrough including dev log, SQLite dump, screenshots |
references/case_study_cc-switch_pr_1624.md |
Frontend/state-management rebase case study โ async init guards, test coupling, i18n conflicts |
references/pr_checklist.md |
Original consolidated checklist (legacy; phase docs supersede the workflow sections) |
references/project_evaluation.md |
Project health rubric for the discovery step |
references/communication_templates.md |
Issue-claim, review-response, and after-merge templates |
references/high_quality_pr_case_study.md |
OpenClaw PR #39763 walkthrough โ small-fix case study |
These are the failure modes that close PRs even when the underlying code is fine. Each one comes from a real PR.
pnpm dev" when you actually only ran the unit tests. A maintainer will try it and lose trust permanently.--fixup commit on the refactor.--lease mid-review. Destroys review threads silently.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="..."
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
Based on successful contributions to active projects:
If your PR misses two or more of these by a lot, re-read Phase 1 before submitting.