mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-13 06:51:27 -10:00
euxaristia
1e73ecae8a
* docs(agents): codify recurring review blockers in AGENTS.md Capture the high-frequency classes that drive multi-round maintainer reviews (stale bases, path canonicalization, secret handling, partial setup, missing regression tests, scope honesty) so coding agents fix them before first review. Refs #856 * docs(agents): tighten security and atomic-write review guidance Require open-time containment binding instead of pre-open EvalSymlinks, and require temp-file then atomic replace rather than exclusive create alone. Refs #856 * docs(agents): require traversal-wide containment for no-follow APIs Final-component-only O_NOFOLLOW is insufficient on Linux and reparse points need platform protections. Align the Security edges guidance with the full-path containment requirement from review.
106 lines
5.8 KiB
Markdown
106 lines
5.8 KiB
Markdown
# Repository Guidelines for Zero
|
|
|
|
These instructions apply to all work in this repository. For the user-facing
|
|
guide to extending Zero with specialists, hooks, plugins, MCP, and skills, see
|
|
[docs/EXTENDING.md](docs/EXTENDING.md).
|
|
|
|
## 1. Contribution and Pull Request Rules
|
|
|
|
- Before opening any pull request, **all contributors**—including maintainers,
|
|
community contributors, and coding agents—must read and follow
|
|
[CONTRIBUTING.md](CONTRIBUTING.md).
|
|
- Community pull requests require an existing parent issue with the
|
|
`issue-approved` label. Team members may open pull requests through the
|
|
internal development process described in `CONTRIBUTING.md`.
|
|
- Keep each change focused on the approved or assigned scope. Do not include
|
|
unrelated fixes, refactors, formatting churn, generated output, or existing
|
|
local changes in the same commit or pull request.
|
|
- Discuss new implementation languages, runtimes, major dependency changes,
|
|
and broad architectural rewrites with maintainers before implementation.
|
|
- Pull request descriptions must explain what changed and why, link the parent
|
|
issue when required, and list the tests or verification performed. Include
|
|
screenshots or a short recording for user-visible UI changes when practical.
|
|
|
|
## 2. Repository and Implementation Conventions
|
|
|
|
- Use the Go version declared in `go.mod`. Do not hardcode a different local
|
|
toolchain version in scripts or documentation.
|
|
- Use the repository build and release commands (`make` and
|
|
`go run ./cmd/zero-release ...`) instead of inventing parallel build flows.
|
|
- Keep tests beside their source files (`foo_test.go` next to `foo.go`). Add a
|
|
regression test for behavior changes and run affected concurrent code under
|
|
the race detector.
|
|
- Never edit files under `third_party/`; they are vendored.
|
|
- Prefer one cross-platform function with small conditional checks over
|
|
duplicated platform-specific helpers when the behavior can remain unified.
|
|
- Do not commit generated benchmark reports from
|
|
`internal/perfbench/reports/*.json`; reports are configuration-specific
|
|
evidence, not repository state.
|
|
- Preserve the user's working tree. Do not overwrite, delete, stage, or commit
|
|
unrelated tracked or untracked files.
|
|
|
|
## 3. Required Validation
|
|
|
|
Run validation from the repository root before committing, opening a pull
|
|
request, or completing an implementation task:
|
|
|
|
1. **Formatting check**: `make fmt-check`. If it fails, format with
|
|
`go fmt ./...` (or `make fmt`) and run the check again.
|
|
2. **Vet**: `go vet ./...` (or `make vet`).
|
|
3. **Tests**: `go test ./...`. Use `make test` for the full race-enabled suite,
|
|
or run focused tests with `-race`, when concurrency is affected.
|
|
4. **Build**: `go run ./cmd/zero-release build`.
|
|
5. **Smoke test**: `go run ./cmd/zero-release smoke`.
|
|
6. **Advisory lint**: `make lint-static`.
|
|
7. **Security**: `make vulncheck`.
|
|
8. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged
|
|
tracked changes).
|
|
|
|
`make lint` currently runs the formatting check and `go vet`; it does **not**
|
|
run golangci-lint. The pinned golangci-lint job is advisory in CI while the
|
|
existing repository-wide backlog is cleaned up. Fix findings introduced by or
|
|
related to the current change. Unrelated pre-existing advisory findings do not
|
|
justify expanding a focused pull request; report them separately.
|
|
|
|
Formatting, vet, tests, build, smoke, diff hygiene, and govulncheck are hard
|
|
requirements. If a related check fails, fix it. If a required check cannot run
|
|
because of the environment or fails for an unrelated external reason, report
|
|
the exact failure and obtain maintainer direction rather than silently ignoring
|
|
it.
|
|
|
|
## 4. Common Review Blockers
|
|
|
|
These classes drive multi-round reviews. Fix them before requesting review:
|
|
|
|
- **Fresh base:** Rebase onto the current PR base (`main` or stacked target)
|
|
before review. A stale head that rolls back mainline commits is a hard
|
|
blocker, not a merge-time detail. Resolve conflicts by keeping upstream
|
|
behavior unless the PR intentionally changes it.
|
|
- **Platform truth:** Code and tests must pass on Linux, macOS, and Windows.
|
|
Compare paths after canonicalization (physical/`EvalSymlinks` form); never
|
|
assert raw `t.TempDir()` spellings (`/var` vs `/private/var` on macOS, short
|
|
paths on Windows). Prefer one cross-platform function with small `GOOS`
|
|
checks over duplicated helpers that drift.
|
|
- **Security edges:** Keep secrets out of argv, env dumps, and logs. Redact
|
|
success and error paths (including stderr). Fail closed on ownership, lease,
|
|
and permission checks. Do not rely on pre-open path resolution
|
|
(`EvalSymlinks` then open) for containment: that is a check-to-use race.
|
|
Bind containment at open/use time with rooted or handle-relative,
|
|
traversal-resistant APIs. If a no-follow API is used, apply it to every
|
|
traversed component and enforce the platform's reparse-point protections;
|
|
final-component-only no-follow is insufficient. On multi-step setup, roll back
|
|
only what this run created; never destroy pre-existing resources you did
|
|
not create; never report success when cleanup or unlock failed.
|
|
- **Atomic shared state:** Write a complete temporary file, then atomically
|
|
replace the destination so concurrent readers never see a partial write.
|
|
Exclusive create or a write lock alone is not enough for readers. Serialize
|
|
the full read-modify-write sequence for lockfiles and shared stores.
|
|
- **Tests match the claim:** Every behavior or security-boundary change needs a
|
|
regression test, including the failure path. Path-sensitive logic needs at
|
|
least one non-Linux path case (or a hermetic fake that exercises the same
|
|
normalization).
|
|
- **Honest scope:** PR description, help text, and comments must match what
|
|
shipped. Wire advertised entry points or shrink the claim. Do not bundle
|
|
unrelated fixes. Do not widen security allowlists by name alone; check
|
|
classification and side effects too.
|