docs: strengthen collaboration governance and AGENTS engineering protocol (#263)
* docs: harden collaboration policy and review automation * ci(docs): remove unsupported lychee --exclude-mail flag * docs(governance): reduce automation side-effects and tighten risk controls * docs(governance): add backlog pruning and supersede protocol * docs(agents): codify engineering principles and risk-tier workflow * docs(readme): add centered star history section at bottom * docs(agents): enforce privacy-safe and neutral test wording * docs(governance): enforce privacy-safe and neutral collaboration checks * fix(ci): satisfy rustfmt and discord schema test fields * docs(governance): require ZeroClaw-native identity wording * docs(agents): add ZeroClaw identity-safe naming palette * docs(governance): codify code naming and architecture contracts * docs(contributing): add naming and architecture good/bad examples * docs(pr): reduce checkbox TODOs and shift to label-first metadata * docs(pr): remove duplicate collaboration track field * ci(labeler): auto-derive module labels and expand provider hints * ci(labeler): auto-apply trusted contributor on PRs and issues * fix(ci): apply rustfmt updates from latest main * ci(labels): flatten namespaces and add contributor tiers * chore: drop stale rustfmt-only drift * ci: scope Rust and docs checks by change set * ci: exclude non-markdown docs from docs-quality targets * ci: satisfy actionlint shellcheck output style * ci(labels): auto-correct manual contributor tier edits * ci(labeler): auto-correct risk label edits * ci(labeler): auto-correct size label edits --------- Co-authored-by: Chummy <183474434+chumyin@users.noreply.github.com>
This commit is contained in:
parent
b5d9f72023
commit
6d56a040ce
16 changed files with 1635 additions and 154 deletions
|
|
@ -9,7 +9,7 @@ Merge-blocking checks should stay small and deterministic. Optional checks are u
|
|||
### Merge-Blocking
|
||||
|
||||
- `.github/workflows/ci.yml` (`CI`)
|
||||
- Purpose: Rust validation (`fmt`, `clippy`, `test`, release build smoke)
|
||||
- Purpose: Rust validation (`fmt`, `clippy`, `test`, release build smoke) + docs quality checks when docs change
|
||||
- Merge gate: `CI Required Gate`
|
||||
- `.github/workflows/workflow-sanity.yml` (`Workflow Sanity`)
|
||||
- Purpose: lint GitHub workflow files (`actionlint`, tab checks)
|
||||
|
|
@ -27,24 +27,35 @@ Merge-blocking checks should stay small and deterministic. Optional checks are u
|
|||
### Optional Repository Automation
|
||||
|
||||
- `.github/workflows/labeler.yml` (`PR Labeler`)
|
||||
- Purpose: path labels + size labels
|
||||
- Purpose: scope/path labels + size/risk labels + fine-grained module labels (`<module>:<component>`)
|
||||
- Additional behavior: provider-related keywords in provider/config/onboard/integration changes are promoted to `provider:*` labels (for example `provider:kimi`, `provider:deepseek`)
|
||||
- Additional behavior: applies contributor tiers on PRs by merged PR count (`experienced` >=10, `principal` >=20, `distinguished` >=50)
|
||||
- Additional behavior: risk + size labels are auto-corrected on manual PR label edits (`labeled`/`unlabeled` events); apply `risk: manual` when maintainers intentionally override automated risk selection
|
||||
- High-risk heuristic paths: `src/security/**`, `src/runtime/**`, `src/gateway/**`, `src/tools/**`, `.github/workflows/**`
|
||||
- Guardrail: maintainers can apply `risk: manual` to freeze automated risk recalculation
|
||||
- `.github/workflows/auto-response.yml` (`Auto Response`)
|
||||
- Purpose: first-time contributor onboarding messages
|
||||
- Purpose: first-time contributor onboarding + label-driven response routing (`r:support`, `r:needs-repro`, etc.)
|
||||
- Additional behavior: applies contributor tiers on issues by merged PR count (`experienced` >=10, `principal` >=20, `distinguished` >=50)
|
||||
- Additional behavior: contributor-tier labels are treated as automation-managed (manual add/remove on PR/issue is auto-corrected)
|
||||
- Guardrail: label-based close routes are issue-only; PRs are never auto-closed by route labels
|
||||
- `.github/workflows/stale.yml` (`Stale`)
|
||||
- Purpose: stale issue/PR lifecycle automation
|
||||
- `.github/dependabot.yml` (`Dependabot`)
|
||||
- Purpose: grouped, rate-limited dependency update PRs (Cargo + GitHub Actions)
|
||||
- `.github/workflows/pr-hygiene.yml` (`PR Hygiene`)
|
||||
- Purpose: nudge stale-but-active PRs to rebase/re-run required checks before queue starvation
|
||||
|
||||
## Trigger Map
|
||||
|
||||
- `CI`: push to `main`/`develop`, PRs to `main`
|
||||
- `CI`: push to `main`, PRs to `main`
|
||||
- `Docker`: push to `main`, tag push (`v*`), PRs touching docker/workflow files, manual dispatch
|
||||
- `Release`: tag push (`v*`)
|
||||
- `Security Audit`: push to `main`, PRs to `main`, weekly schedule
|
||||
- `Workflow Sanity`: PR/push when `.github/workflows/**`, `.github/*.yml`, or `.github/*.yaml` change
|
||||
- `PR Labeler`: `pull_request_target` lifecycle events
|
||||
- `Auto Response`: issue opened, `pull_request_target` opened
|
||||
- `Auto Response`: issue opened/labeled, `pull_request_target` opened/labeled
|
||||
- `Stale`: daily schedule, manual dispatch
|
||||
- `Dependabot`: weekly dependency maintenance windows
|
||||
- `PR Hygiene`: every 12 hours schedule, manual dispatch
|
||||
|
||||
## Fast Triage Guide
|
||||
|
|
@ -54,10 +65,21 @@ Merge-blocking checks should stay small and deterministic. Optional checks are u
|
|||
3. Release failures on tags: inspect `.github/workflows/release.yml`.
|
||||
4. Security failures: inspect `.github/workflows/security.yml` and `deny.toml`.
|
||||
5. Workflow syntax/lint failures: inspect `.github/workflows/workflow-sanity.yml`.
|
||||
6. Docs failures in CI: inspect `docs-quality` job logs in `.github/workflows/ci.yml`.
|
||||
|
||||
## Maintenance Rules
|
||||
|
||||
- Keep merge-blocking checks deterministic and reproducible (`--locked` where applicable).
|
||||
- Prefer explicit workflow permissions (least privilege).
|
||||
- Use path filters for expensive workflows when practical.
|
||||
- Keep docs quality checks low-noise (`markdownlint` + offline link checks).
|
||||
- Keep dependency update volume controlled (grouping + PR limits).
|
||||
- Avoid mixing onboarding/community automation with merge-gating logic.
|
||||
|
||||
## Automation Side-Effect Controls
|
||||
|
||||
- Prefer deterministic automation that can be manually overridden (`risk: manual`) when context is nuanced.
|
||||
- Keep auto-response comments deduplicated to prevent triage noise.
|
||||
- Keep auto-close behavior scoped to issues; maintainers own PR close/merge decisions.
|
||||
- If automation is wrong, correct labels first, then continue review with explicit rationale.
|
||||
- Use `superseded` / `stale-candidate` labels to prune duplicate or dormant PRs before deep review.
|
||||
|
|
|
|||
|
|
@ -9,7 +9,10 @@ This document defines how ZeroClaw handles high PR volume while maintaining:
|
|||
- High sustainability
|
||||
- High security
|
||||
|
||||
Related reference: [`docs/ci-map.md`](ci-map.md) for per-workflow ownership, triggers, and triage flow.
|
||||
Related references:
|
||||
|
||||
- [`docs/ci-map.md`](ci-map.md) for per-workflow ownership, triggers, and triage flow.
|
||||
- [`docs/reviewer-playbook.md`](reviewer-playbook.md) for day-to-day reviewer execution.
|
||||
|
||||
## 1) Governance Goals
|
||||
|
||||
|
|
@ -17,6 +20,18 @@ Related reference: [`docs/ci-map.md`](ci-map.md) for per-workflow ownership, tri
|
|||
2. Keep CI signal quality high (fast feedback, low false positives).
|
||||
3. Keep security review explicit for risky surfaces.
|
||||
4. Keep changes easy to reason about and easy to revert.
|
||||
5. Keep repository artifacts free of personal/sensitive data leakage.
|
||||
|
||||
### Governance Design Logic (Control Loop)
|
||||
|
||||
This workflow is intentionally layered to reduce reviewer load while keeping accountability clear:
|
||||
|
||||
1. **Intake classification**: path/size/risk/module labels route the PR to the right review depth.
|
||||
2. **Deterministic validation**: merge gate depends on reproducible checks, not subjective comments.
|
||||
3. **Risk-based review depth**: high-risk paths trigger deep review; low-risk paths stay fast.
|
||||
4. **Rollback-first merge contract**: every merge path includes concrete recovery steps.
|
||||
|
||||
Automation assists with triage and guardrails, but final merge accountability remains with human maintainers and PR authors.
|
||||
|
||||
## 2) Required Repository Settings
|
||||
|
||||
|
|
@ -34,8 +49,8 @@ Maintain these branch protection rules on `main`:
|
|||
### Step A: Intake
|
||||
|
||||
- Contributor opens PR with full `.github/pull_request_template.md`.
|
||||
- `PR Labeler` applies path labels + size labels.
|
||||
- `Auto Response` posts first-time contributor guidance.
|
||||
- `PR Labeler` applies scope/path labels + size labels + risk labels + module labels (for example `channel:telegram`, `provider:kimi`, `tool:shell`) and contributor tiers by merged PR count (`experienced` >=10, `principal` >=20, `distinguished` >=50).
|
||||
- `Auto Response` posts first-time guidance and handles label-driven routing for low-signal items.
|
||||
|
||||
### Step B: Validation
|
||||
|
||||
|
|
@ -46,7 +61,7 @@ Maintain these branch protection rules on `main`:
|
|||
### Step C: Review
|
||||
|
||||
- Reviewers prioritize by risk and size labels.
|
||||
- Security-sensitive paths (`src/security`, runtime, CI) require maintainer attention.
|
||||
- Security-sensitive paths (`src/security`, `src/runtime`, `src/gateway`, and CI workflows) require maintainer attention.
|
||||
- Large PRs (`size: L`/`size: XL`) should be split unless strongly justified.
|
||||
|
||||
### Step D: Merge
|
||||
|
|
@ -55,7 +70,26 @@ Maintain these branch protection rules on `main`:
|
|||
- PR title should follow Conventional Commit style.
|
||||
- Merge only when rollback path is documented.
|
||||
|
||||
## 4) PR Size Policy
|
||||
## 4) PR Readiness Contracts (DoR / DoD)
|
||||
|
||||
### Definition of Ready (before requesting review)
|
||||
|
||||
- PR template fully completed.
|
||||
- Scope boundary is explicit (what changed / what did not).
|
||||
- Validation evidence attached (not just "CI will check").
|
||||
- Security and rollback fields completed for risky paths.
|
||||
- Privacy/data-hygiene checks are completed and test language is neutral/project-scoped.
|
||||
- If identity-like wording appears in tests/examples, it is normalized to ZeroClaw/project-native labels.
|
||||
|
||||
### Definition of Done (merge-ready)
|
||||
|
||||
- `CI Required Gate` is green.
|
||||
- Required reviewers approved (including CODEOWNERS paths).
|
||||
- Risk class labels match touched paths.
|
||||
- Migration/compatibility impact is documented.
|
||||
- Rollback path is concrete and fast.
|
||||
|
||||
## 5) PR Size Policy
|
||||
|
||||
- `size: XS` <= 80 changed lines
|
||||
- `size: S` <= 250 changed lines
|
||||
|
|
@ -69,7 +103,12 @@ Policy:
|
|||
- `L/XL` PRs need explicit justification and tighter test evidence.
|
||||
- If a large feature is unavoidable, split into stacked PRs.
|
||||
|
||||
## 5) AI/Agent Contribution Policy
|
||||
Automation behavior:
|
||||
|
||||
- `PR Labeler` applies `size:*` labels from effective changed lines.
|
||||
- Docs-only/lockfile-heavy PRs are normalized to avoid size inflation.
|
||||
|
||||
## 6) AI/Agent Contribution Policy
|
||||
|
||||
AI-assisted PRs are welcome, and review can also be agent-assisted.
|
||||
|
||||
|
|
@ -93,22 +132,43 @@ Review emphasis for AI-heavy PRs:
|
|||
- Error handling and fallback behavior
|
||||
- Performance and memory regressions
|
||||
|
||||
## 6) Review SLA and Queue Discipline
|
||||
## 7) Review SLA and Queue Discipline
|
||||
|
||||
- First maintainer triage target: within 48 hours.
|
||||
- If PR is blocked, maintainer leaves one actionable checklist.
|
||||
- `stale` automation is used to keep queue healthy; maintainers can apply `no-stale` when needed.
|
||||
- `pr-hygiene` automation checks open PRs every 12 hours and posts a nudge when a PR has no new commits for 48+ hours and is either behind `main` or missing/failing `CI Required Gate` on the head commit.
|
||||
|
||||
## 7) Security and Stability Rules
|
||||
Backlog pressure controls:
|
||||
|
||||
- Use a review queue budget: limit concurrent deep-review PRs per maintainer and keep the rest in triage state.
|
||||
- For stacked work, require explicit `Depends on #...` so review order is deterministic.
|
||||
- If a new PR replaces an older open PR, require `Supersedes #...` and close the older one after maintainer confirmation.
|
||||
- Mark dormant/redundant PRs with `stale-candidate` or `superseded` to reduce duplicate review effort.
|
||||
|
||||
Issue triage discipline:
|
||||
|
||||
- `r:needs-repro` for incomplete bug reports (request deterministic repro before deep triage).
|
||||
- `r:support` for usage/help items better handled outside bug backlog.
|
||||
- `invalid` / `duplicate` labels trigger **issue-only** closing automation with guidance.
|
||||
|
||||
Automation side-effect guards:
|
||||
|
||||
- `Auto Response` deduplicates label-based comments to avoid spam.
|
||||
- Automated close routes are limited to issues, not PRs.
|
||||
- Maintainers can freeze automated risk recalculation with `risk: manual` when context demands human override.
|
||||
|
||||
## 8) Security and Stability Rules
|
||||
|
||||
Changes in these areas require stricter review and stronger test evidence:
|
||||
|
||||
- `src/security/**`
|
||||
- runtime process management
|
||||
- gateway ingress/authentication behavior (`src/gateway/**`)
|
||||
- filesystem access boundaries
|
||||
- network/authentication behavior
|
||||
- GitHub workflows and release pipeline
|
||||
- tools with execution capability (`src/tools/**`)
|
||||
|
||||
Minimum for risky PRs:
|
||||
|
||||
|
|
@ -116,7 +176,14 @@ Minimum for risky PRs:
|
|||
- mitigation notes
|
||||
- rollback steps
|
||||
|
||||
## 8) Failure Recovery
|
||||
Recommended for high-risk PRs:
|
||||
|
||||
- include a focused test proving boundary behavior
|
||||
- include one explicit failure-mode scenario and expected degradation
|
||||
|
||||
For agent-assisted contributions, reviewers should also verify the author demonstrates understanding of runtime behavior and blast radius.
|
||||
|
||||
## 9) Failure Recovery
|
||||
|
||||
If a merged PR causes regressions:
|
||||
|
||||
|
|
@ -126,16 +193,18 @@ If a merged PR causes regressions:
|
|||
|
||||
Prefer fast restore of service quality over delayed perfect fixes.
|
||||
|
||||
## 9) Maintainer Checklist (Merge-Ready)
|
||||
## 10) Maintainer Checklist (Merge-Ready)
|
||||
|
||||
- Scope is focused and understandable.
|
||||
- CI gate is green.
|
||||
- Docs-quality checks are green when docs changed.
|
||||
- Security impact fields are complete.
|
||||
- Privacy/data-hygiene fields are complete and evidence is redacted/anonymized.
|
||||
- Agent workflow notes are sufficient for reproducibility (if automation was used).
|
||||
- Rollback plan is explicit.
|
||||
- Commit title follows Conventional Commits.
|
||||
|
||||
## 10) Agent Review Operating Model
|
||||
## 11) Agent Review Operating Model
|
||||
|
||||
To keep review quality stable under high PR volume, we use a two-lane review model:
|
||||
|
||||
|
|
@ -145,6 +214,8 @@ To keep review quality stable under high PR volume, we use a two-lane review mod
|
|||
- Confirm CI gate signal (`CI Required Gate`).
|
||||
- Confirm risk class via labels and touched paths.
|
||||
- Confirm rollback statement exists.
|
||||
- Confirm privacy/data-hygiene section and neutral wording requirements are satisfied.
|
||||
- Confirm any required identity-like wording uses ZeroClaw/project-native terminology.
|
||||
|
||||
### Lane B: Deep review (risk-based)
|
||||
|
||||
|
|
@ -155,7 +226,7 @@ Required for high-risk changes (security/runtime/gateway/CI):
|
|||
- Validate backward compatibility and migration impact.
|
||||
- Validate observability/logging impact.
|
||||
|
||||
## 11) Queue Priority and Label Discipline
|
||||
## 12) Queue Priority and Label Discipline
|
||||
|
||||
Triage order recommendation:
|
||||
|
||||
|
|
@ -167,9 +238,12 @@ Label discipline:
|
|||
|
||||
- Path labels identify subsystem ownership quickly.
|
||||
- Size labels drive batching strategy.
|
||||
- Risk labels drive review depth (`risk: low/medium/high`).
|
||||
- Module labels (`<module>:<component>`) improve reviewer routing for integration-specific changes and future newly-added modules.
|
||||
- `risk: manual` allows maintainers to preserve a human risk judgment when automation lacks context.
|
||||
- `no-stale` is reserved for accepted-but-blocked work.
|
||||
|
||||
## 12) Agent Handoff Contract
|
||||
## 13) Agent Handoff Contract
|
||||
|
||||
When one agent hands off to another (or to a maintainer), include:
|
||||
|
||||
|
|
|
|||
110
docs/reviewer-playbook.md
Normal file
110
docs/reviewer-playbook.md
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
# Reviewer Playbook
|
||||
|
||||
This playbook is the operational companion to [`docs/pr-workflow.md`](pr-workflow.md).
|
||||
Use it to reduce review latency without reducing quality.
|
||||
|
||||
## 1) Review Objectives
|
||||
|
||||
- Keep queue throughput predictable.
|
||||
- Keep risk review proportionate to change risk.
|
||||
- Keep merge decisions reproducible and auditable.
|
||||
|
||||
## 2) 5-Minute Intake Triage
|
||||
|
||||
For every new PR, do a fast intake pass:
|
||||
|
||||
1. Confirm template completeness (`summary`, `validation`, `security`, `rollback`).
|
||||
2. Confirm labels (`size:*`, `risk:*`, scope labels such as `provider`/`channel`/`security`, module-scoped labels such as `channel:*`/`provider:*`/`tool:*`, and contributor tier labels when applicable) are present and plausible.
|
||||
3. Confirm CI signal status (`CI Required Gate`).
|
||||
4. Confirm scope is one concern (reject mixed mega-PRs unless justified).
|
||||
5. Confirm privacy/data-hygiene and neutral test wording requirements are satisfied.
|
||||
|
||||
If any intake requirement fails, leave one actionable checklist comment instead of deep review.
|
||||
|
||||
## 3) Risk-to-Depth Matrix
|
||||
|
||||
| Risk label | Typical touched paths | Minimum review depth |
|
||||
|---|---|---|
|
||||
| `risk: low` | docs/tests/chore, isolated non-runtime changes | 1 reviewer + CI gate |
|
||||
| `risk: medium` | `src/providers/**`, `src/channels/**`, `src/memory/**`, `src/config/**` | 1 subsystem-aware reviewer + behavior verification |
|
||||
| `risk: high` | `src/security/**`, `src/runtime/**`, `src/gateway/**`, `src/tools/**`, `.github/workflows/**` | fast triage + deep review, strong rollback and failure-mode checks |
|
||||
|
||||
When uncertain, treat as `risk: high`.
|
||||
|
||||
If automated risk labeling is contextually wrong, maintainers can apply `risk: manual` and set the final risk label explicitly.
|
||||
|
||||
## 4) Fast-Lane Checklist (All PRs)
|
||||
|
||||
- Scope boundary is explicit and believable.
|
||||
- Validation commands are present and results are coherent.
|
||||
- User-facing behavior changes are documented.
|
||||
- Author demonstrates understanding of behavior and blast radius (especially for agent-assisted PRs).
|
||||
- Rollback path is concrete (not just “revert”).
|
||||
- Compatibility/migration impacts are clear.
|
||||
- No personal/sensitive data leakage in diff artifacts; examples/tests remain neutral and project-scoped.
|
||||
- If identity-like wording exists, it uses ZeroClaw/project-native roles (not personal or real-world identities).
|
||||
- Naming and architecture boundaries follow project contracts (`AGENTS.md`, `CONTRIBUTING.md`).
|
||||
|
||||
## 5) Deep Review Checklist (High Risk)
|
||||
|
||||
For high-risk PRs, verify at least one example in each category:
|
||||
|
||||
- **Security boundaries**: deny-by-default behavior preserved, no accidental scope broadening.
|
||||
- **Failure modes**: error handling is explicit and degrades safely.
|
||||
- **Contract stability**: CLI/config/API compatibility preserved or migration documented.
|
||||
- **Observability**: failures are diagnosable without leaking secrets.
|
||||
- **Rollback safety**: revert path and blast radius are clear.
|
||||
|
||||
## 6) Issue Triage Playbook
|
||||
|
||||
Use labels to keep backlog actionable:
|
||||
|
||||
- `r:needs-repro` for incomplete bug reports.
|
||||
- `r:support` for usage/support questions better routed outside bug backlog.
|
||||
- `duplicate` / `invalid` for non-actionable duplicates/noise.
|
||||
- `no-stale` for accepted work waiting on external blockers.
|
||||
- Request redaction if logs/payloads include personal identifiers or sensitive data.
|
||||
|
||||
## 7) Review Comment Style
|
||||
|
||||
Prefer checklist-style comments with one of these outcomes:
|
||||
|
||||
- **Ready to merge** (explicitly say why).
|
||||
- **Needs author action** (ordered list of blockers).
|
||||
- **Needs deeper security/runtime review** (state exact risk and requested evidence).
|
||||
|
||||
Avoid vague comments that create back-and-forth latency.
|
||||
|
||||
## 8) Automation Override Protocol
|
||||
|
||||
Use this when automation output creates review side effects:
|
||||
|
||||
1. **Incorrect risk label**: add `risk: manual`, then set the intended `risk:*` label.
|
||||
2. **Incorrect auto-close on issue triage**: reopen issue, remove route label, and leave one clarifying comment.
|
||||
3. **Label spam/noise**: keep one canonical maintainer comment and remove redundant route labels.
|
||||
4. **Ambiguous PR scope**: request split before deep review.
|
||||
|
||||
### PR Backlog Pruning Protocol
|
||||
|
||||
When review demand exceeds capacity, apply this order:
|
||||
|
||||
1. Keep active bug/security PRs (`size: XS/S`) at the top of queue.
|
||||
2. Ask overlapping PRs to consolidate; close older ones as `superseded` after acknowledgement.
|
||||
3. Mark dormant PRs as `stale-candidate` before stale closure window starts.
|
||||
4. Require rebase + fresh validation before reopening stale/superseded technical work.
|
||||
|
||||
## 9) Handoff Protocol
|
||||
|
||||
If handing off review to another maintainer/agent, include:
|
||||
|
||||
1. Scope summary
|
||||
2. Current risk class and why
|
||||
3. What has been validated already
|
||||
4. Open blockers
|
||||
5. Suggested next action
|
||||
|
||||
## 10) Weekly Queue Hygiene
|
||||
|
||||
- Review stale queue and apply `no-stale` only to accepted-but-blocked work.
|
||||
- Prioritize `size: XS/S` bug/security PRs first.
|
||||
- Convert recurring support issues into docs updates and auto-response guidance.
|
||||
Loading…
Add table
Add a link
Reference in a new issue