|
| 1 | +# Contributing to PicoClaw |
| 2 | + |
| 3 | +Thank you for your interest in contributing to PicoClaw! This project is a community-driven effort to build the lightweight and versatile personal AI assistant. We welcome contributions of all kinds: bug fixes, features, documentation, translations, and testing. |
| 4 | + |
| 5 | +PicoClaw itself was substantially developed with AI assistance — we embrace this approach and have built our contribution process around it. |
| 6 | + |
| 7 | +## Table of Contents |
| 8 | + |
| 9 | +- [Code of Conduct](#code-of-conduct) |
| 10 | +- [Ways to Contribute](#ways-to-contribute) |
| 11 | +- [Getting Started](#getting-started) |
| 12 | +- [Development Setup](#development-setup) |
| 13 | +- [Making Changes](#making-changes) |
| 14 | +- [AI-Assisted Contributions](#ai-assisted-contributions) |
| 15 | +- [Pull Request Process](#pull-request-process) |
| 16 | +- [Branch Strategy](#branch-strategy) |
| 17 | +- [Code Review](#code-review) |
| 18 | +- [Communication](#communication) |
| 19 | + |
| 20 | +--- |
| 21 | + |
| 22 | +## Code of Conduct |
| 23 | + |
| 24 | +We are committed to maintaining a welcoming and respectful community. Be kind, constructive, and assume good faith. Harassment or discrimination of any kind will not be tolerated. |
| 25 | + |
| 26 | +--- |
| 27 | + |
| 28 | +## Ways to Contribute |
| 29 | + |
| 30 | +- **Bug reports** — Open an issue using the bug report template. |
| 31 | +- **Feature requests** — Open an issue using the feature request template; discuss before implementing. |
| 32 | +- **Code** — Fix bugs or implement features. See the workflow below. |
| 33 | +- **Documentation** — Improve READMEs, docs, inline comments, or translations. |
| 34 | +- **Testing** — Run PicoClaw on new hardware, channels, or LLM providers and report your results. |
| 35 | + |
| 36 | +For substantial new features, please open an issue first to discuss the design before writing code. This prevents wasted effort and ensures alignment with the project's direction. |
| 37 | + |
| 38 | +--- |
| 39 | + |
| 40 | +## Getting Started |
| 41 | + |
| 42 | +1. **Fork** the repository on GitHub. |
| 43 | +2. **Clone** your fork locally: |
| 44 | + ```bash |
| 45 | + git clone https://github.com/<your-username>/picoclaw.git |
| 46 | + cd picoclaw |
| 47 | + ``` |
| 48 | +3. Add the upstream remote: |
| 49 | + ```bash |
| 50 | + git remote add upstream https://github.com/sipeed/picoclaw.git |
| 51 | + ``` |
| 52 | + |
| 53 | +--- |
| 54 | + |
| 55 | +## Development Setup |
| 56 | + |
| 57 | +### Prerequisites |
| 58 | + |
| 59 | +- Go 1.25 or later |
| 60 | +- `make` |
| 61 | + |
| 62 | +### Build |
| 63 | + |
| 64 | +```bash |
| 65 | +make build # Build binary (runs go generate first) |
| 66 | +make generate # Run go generate only |
| 67 | +make check # Full pre-commit check: deps + fmt + vet + test |
| 68 | +``` |
| 69 | + |
| 70 | +### Running Tests |
| 71 | + |
| 72 | +```bash |
| 73 | +make test # Run all tests |
| 74 | +go test -run TestName -v ./pkg/session/ # Run a single test |
| 75 | +go test -bench=. -benchmem -run='^$' ./... # Run benchmarks |
| 76 | +``` |
| 77 | + |
| 78 | +### Code Style |
| 79 | + |
| 80 | +```bash |
| 81 | +make fmt # Format code |
| 82 | +make vet # Static analysis |
| 83 | +make lint # Full linter run |
| 84 | +``` |
| 85 | + |
| 86 | +All CI checks must pass before a PR can be merged. Run `make check` locally before pushing to catch issues early. |
| 87 | + |
| 88 | +--- |
| 89 | + |
| 90 | +## Making Changes |
| 91 | + |
| 92 | +### Branching |
| 93 | + |
| 94 | +Always branch off `main` and target `main` in your PR. Never push directly to `main` or any `release/*` branch: |
| 95 | + |
| 96 | +```bash |
| 97 | +git checkout main |
| 98 | +git pull upstream main |
| 99 | +git checkout -b your-feature-branch |
| 100 | +``` |
| 101 | + |
| 102 | +Use descriptive branch names, e.g. `fix/telegram-timeout`, `feat/ollama-provider`, `docs/contributing-guide`. |
| 103 | + |
| 104 | +### Commits |
| 105 | + |
| 106 | +- Write clear, concise commit messages in English. |
| 107 | +- Use the imperative mood: "Add retry logic" not "Added retry logic". |
| 108 | +- Reference the related issue when relevant: `Fix session leak (#123)`. |
| 109 | +- Keep commits focused. One logical change per commit is preferred. |
| 110 | +- For minor cleanups or typo fixes, squash them into a single commit before opening a PR. |
| 111 | +- Refer to https://www.conventionalcommits.org/zh-hans/v1.0.0/ |
| 112 | + |
| 113 | +### Keeping Up to Date |
| 114 | + |
| 115 | +Rebase your branch onto upstream `main` before opening a PR: |
| 116 | + |
| 117 | +```bash |
| 118 | +git fetch upstream |
| 119 | +git rebase upstream/main |
| 120 | +``` |
| 121 | + |
| 122 | +--- |
| 123 | + |
| 124 | +## AI-Assisted Contributions |
| 125 | + |
| 126 | +PicoClaw was built with substantial AI assistance, and we fully embrace AI-assisted development. However, contributors must understand their responsibilities when using AI tools. |
| 127 | + |
| 128 | +### Disclosure Is Required |
| 129 | + |
| 130 | +Every PR must disclose AI involvement using the PR template's **🤖 AI Code Generation** section. There are three levels: |
| 131 | + |
| 132 | +| Level | Description | |
| 133 | +|---|---| |
| 134 | +| 🤖 Fully AI-generated | AI wrote the code; contributor reviewed and validated it | |
| 135 | +| 🛠️ Mostly AI-generated | AI produced the draft; contributor made significant modifications | |
| 136 | +| 👨💻 Mostly Human-written | Contributor led; AI provided suggestions or none at all | |
| 137 | + |
| 138 | +Honest disclosure is expected. There is no stigma attached to any level — what matters is the quality of the contribution. |
| 139 | + |
| 140 | +### You Are Responsible for What You Submit |
| 141 | + |
| 142 | +Using AI to generate code does not reduce your responsibility as the contributor. Before opening a PR with AI-generated code, you must: |
| 143 | + |
| 144 | +- **Read and understand** every line of the generated code. |
| 145 | +- **Test it** in a real environment (see the Test Environment section of the PR template). |
| 146 | +- **Check for security issues** — AI models can generate subtly insecure code (e.g., path traversal, injection, credential exposure). Review carefully. |
| 147 | +- **Verify correctness** — AI-generated logic can be plausible-sounding but wrong. Validate the behavior, not just the syntax. |
| 148 | + |
| 149 | +PRs where it is clear the contributor has not read or tested the AI-generated code will be closed without review. |
| 150 | + |
| 151 | +### AI-Generated Code Quality Standards |
| 152 | + |
| 153 | +AI-generated contributions are held to the **same quality bar** as human-written code: |
| 154 | + |
| 155 | +- It must pass all CI checks (`make check`). |
| 156 | +- It must be idiomatic Go and consistent with the existing codebase style. |
| 157 | +- It must not introduce unnecessary abstractions, dead code, or over-engineering. |
| 158 | +- It must include or update tests where appropriate. |
| 159 | + |
| 160 | +### Security Review |
| 161 | + |
| 162 | +AI-generated code requires extra security scrutiny. Pay special attention to: |
| 163 | + |
| 164 | +- File path handling and sandbox escapes (see commit `244eb0b` for a real example) |
| 165 | +- External input validation in channel handlers and tool implementations |
| 166 | +- Credential or secret handling |
| 167 | +- Command execution (`exec.Command`, shell invocations) |
| 168 | + |
| 169 | +If you are unsure whether a piece of AI-generated code is safe, say so in the PR — reviewers will help. |
| 170 | + |
| 171 | +--- |
| 172 | + |
| 173 | +## Pull Request Process |
| 174 | + |
| 175 | +### Before Opening a PR |
| 176 | + |
| 177 | +- [ ] Run `make check` and ensure it passes locally. |
| 178 | +- [ ] Fill in the PR template completely, including the AI disclosure section. |
| 179 | +- [ ] Link any related issue(s) in the PR description. |
| 180 | +- [ ] Keep the PR focused. Avoid bundling unrelated changes together. |
| 181 | + |
| 182 | +### PR Template Sections |
| 183 | + |
| 184 | +The PR template asks for: |
| 185 | + |
| 186 | +- **Description** — What does this change do and why? |
| 187 | +- **Type of Change** — Bug fix, feature, docs, or refactor. |
| 188 | +- **AI Code Generation** — Disclosure of AI involvement (required). |
| 189 | +- **Related Issue** — Link to the issue this addresses. |
| 190 | +- **Technical Context** — Reference URLs and reasoning (skip for pure docs PRs). |
| 191 | +- **Test Environment** — Hardware, OS, model/provider, and channels used for testing. |
| 192 | +- **Evidence** — Optional logs or screenshots demonstrating the change works. |
| 193 | +- **Checklist** — Self-review confirmation. |
| 194 | + |
| 195 | +### PR Size |
| 196 | + |
| 197 | +Prefer small, reviewable PRs. A PR that changes 200 lines across 5 files is much easier to review than one that changes 2000 lines across 30 files. If your feature is large, consider splitting it into a series of smaller, logically complete PRs. |
| 198 | + |
| 199 | +--- |
| 200 | + |
| 201 | +## Branch Strategy |
| 202 | + |
| 203 | +### Long-Lived Branches |
| 204 | + |
| 205 | +- **`main`** — the active development branch. All feature PRs target `main`. The branch is protected: direct pushes are not permitted, and at least one maintainer approval is required before merging. |
| 206 | +- **`release/x.y`** — stable release branches, cut from `main` when a version is ready to ship. These branches are more strictly protected than `main`. |
| 207 | + |
| 208 | +### Requirements to Merge into `main` |
| 209 | + |
| 210 | +A PR can only be merged when all of the following are satisfied: |
| 211 | + |
| 212 | +1. **CI passes** — All GitHub Actions workflows (lint, test, build) must be green. |
| 213 | +2. **Reviewer approval** — At least one maintainer has approved the PR. |
| 214 | +3. **No unresolved review comments** — All review threads must be resolved. |
| 215 | +4. **PR template is complete** — Including AI disclosure and test environment. |
| 216 | + |
| 217 | +### Who Can Merge |
| 218 | + |
| 219 | +Only maintainers can merge PRs. Contributors cannot merge their own PRs, even if they have write access. |
| 220 | + |
| 221 | +### Merge Strategy |
| 222 | + |
| 223 | +We use **squash merge** for most PRs to keep the `main` history clean and readable. Each merged PR becomes a single commit referencing the PR number, e.g.: |
| 224 | + |
| 225 | +``` |
| 226 | +feat: Add Ollama provider support (#491) |
| 227 | +``` |
| 228 | + |
| 229 | +If a PR consists of multiple independent, well-separated commits that tell a clear story, a regular merge may be used at the maintainer's discretion. |
| 230 | + |
| 231 | +### Release Branches |
| 232 | + |
| 233 | +When a version is ready, maintainers cut a `release/x.y` branch from `main`. After that point: |
| 234 | + |
| 235 | +- **New features are not backported.** The release branch receives no new functionality after it is cut. |
| 236 | +- **Security fixes and critical bug fixes are cherry-picked.** If a fix in `main` qualifies (security vulnerability, data loss, crash), maintainers will cherry-pick the relevant commit(s) onto the affected `release/x.y` branch and issue a patch release. |
| 237 | + |
| 238 | +If you believe a fix in `main` should be backported to a release branch, note it in the PR description or open a separate issue. The decision rests with the maintainers. |
| 239 | + |
| 240 | +Release branches have stricter protections than `main` and are never directly pushed to under any circumstances. |
| 241 | + |
| 242 | +--- |
| 243 | + |
| 244 | +## Code Review |
| 245 | + |
| 246 | +### For Contributors |
| 247 | + |
| 248 | +- Respond to review comments within a reasonable time. If you need more time, say so. |
| 249 | +- When you update a PR in response to feedback, briefly note what changed (e.g., "Updated to use `sync.RWMutex` as suggested"). |
| 250 | +- If you disagree with feedback, engage respectfully. Explain your reasoning; reviewers can be wrong too. |
| 251 | +- Do not force-push after a review has started — it makes it harder for reviewers to see what changed. Use additional commits instead; the maintainer will squash on merge. |
| 252 | + |
| 253 | +### For Reviewers |
| 254 | + |
| 255 | +Review for: |
| 256 | + |
| 257 | +1. **Correctness** — Does the code do what it claims? Are there edge cases? |
| 258 | +2. **Security** — Especially for AI-generated code, tool implementations, and channel handlers. |
| 259 | +3. **Architecture** — Is the approach consistent with the existing design? |
| 260 | +4. **Simplicity** — Is there a simpler solution? Does this add unnecessary complexity? |
| 261 | +5. **Tests** — Are the changes covered by tests? Are existing tests still meaningful? |
| 262 | + |
| 263 | +Be constructive and specific. "This could have a race condition if two goroutines call this concurrently — consider using a mutex here" is better than "this looks wrong". |
| 264 | + |
| 265 | + |
| 266 | +### Reviewer List |
| 267 | +Once your PR is submitted, you can reach out to the assigned reviewers listed in the following table. |
| 268 | + |
| 269 | +|Function| Reviewer| |
| 270 | +|--- |--- | |
| 271 | +|Provider|@yinwm | |
| 272 | +|Channel |@yinwm | |
| 273 | +|Agent |@lxowalle| |
| 274 | +|Tools |@lxowalle| |
| 275 | +|SKill || |
| 276 | +|MCP || |
| 277 | +|Optimization|@lxowalle| |
| 278 | +|Security|| |
| 279 | +|AI CI |@imguoguo| |
| 280 | +|UX || |
| 281 | +|Document|| |
| 282 | + |
| 283 | +--- |
| 284 | + |
| 285 | +## Communication |
| 286 | + |
| 287 | +- **GitHub Issues** — Bug reports, feature requests, design discussions. |
| 288 | +- **GitHub Discussions** — General questions, ideas, community conversation. |
| 289 | +- **Pull Request comments** — Code-specific feedback. |
| 290 | +- **Wechat&Discord** — We will invite you when you have at least one merged PR |
| 291 | + |
| 292 | +When in doubt, open an issue before writing code. It costs little and prevents wasted effort. |
| 293 | + |
| 294 | +--- |
| 295 | + |
| 296 | +## A Note on the Project's AI-Driven Origin |
| 297 | + |
| 298 | +PicoClaw's architecture was substantially designed and implemented with AI assistance, guided by human oversight. If you find something that looks odd or over-engineered, it may be an artifact of that process — opening an issue to discuss it is always welcome. |
| 299 | + |
| 300 | +We believe AI-assisted development done responsibly produces great results. We also believe humans must remain accountable for what they ship. These two beliefs are not in conflict. |
| 301 | + |
| 302 | +Thank you for contributing! |
0 commit comments