mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
docs(agents): restructure root AGENTS.md around product doctrine and deduped policy
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
# AGENTS.MD
|
||||
|
||||
Telegraph style. Root rules only. Read scoped `AGENTS.md` before subtree work.
|
||||
Skills own workflows; root owns hard policy and routing.
|
||||
Skills own workflows; root owns hard policy and routing. Product direction and merge scope: `VISION.md`.
|
||||
|
||||
## Start
|
||||
|
||||
@@ -22,6 +22,19 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- New channel/plugin/app/doc surface: update `.github/labeler.yml` + GH labels.
|
||||
- New `AGENTS.md`: add sibling `CLAUDE.md` symlink; edit `AGENTS.md` only.
|
||||
|
||||
## Product Doctrine
|
||||
|
||||
`VISION.md` owns direction; this section owns judgment. Apply to triage, review, design, and landing.
|
||||
|
||||
- Judge from the operator's chair: a competent person following the docs must end with a working, comprehensible bot. Code correctness is table stakes, not the verdict.
|
||||
- Severity order: silent failure > crash > missing feature. Every user or agent action ends in a visible outcome or a recorded, intentional non-outcome. An action that produces nothing, with nothing explaining why, is the worst bug class in this repo.
|
||||
- Defaults are the product. Most operators never change them; a regression on a default path outranks feature work and config-path bugs.
|
||||
- Record facts where they happen; read them where they are needed. Answering "did X happen?" by combining several indirect signals rots as sibling paths evolve; prefer a recorded fact at the boundary that owns it.
|
||||
- The model's experience is the product. Capability that prompt/tool text does not mention — or contradicts — does not exist for users. Tool results are prompts: return what the model needs next, not a bare ack. Review prompt and description text with the same rigor as code.
|
||||
- Latency is model round-trips, not milliseconds. Collapse act-then-observe pairs into one tool result; keep expensive resources warm across a session.
|
||||
- Never dead-end the agent: failure text states what to try next; unavailable tools are hidden by gating, not left to fail; missing pieces provision automatically where safe — within the fallback rules in Architecture.
|
||||
- A capability shipped off by default needs a named enablement path (onboarding, doctor hint, preset, or docs surfacing) in the same change. Dark-shipped features are a review smell.
|
||||
|
||||
## ClawSweeper Review Policy
|
||||
|
||||
- OpenClaw-specific review rules live here; generic ClawSweeper prompts stay repo-agnostic.
|
||||
@@ -34,6 +47,8 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- Every PR review must explicitly ask whether the PR is the best fix, not merely a plausible fix. Verdicts need a best-fix judgment backed by enough code reading to compare owner boundaries, callers, siblings, tests, docs, current `main`, shipped behavior when relevant, and dependency/Codex contracts when involved.
|
||||
- Before a PR verdict, build a small evidence map: changed surface, entry point, owner boundary, at least one caller and callee, sibling surfaces that share the invariant, existing tests, and current `main` behavior. If any cell is missing, say the gap instead of concluding.
|
||||
- One-sided fixes need sibling-surface proof, an explanation for why siblings are unaffected, or explicit follow-up work.
|
||||
- Doctrine-class findings are first-class: an action path that can end with no visible outcome and no recorded reason; a default-path regression; prompt/tool-description text that contradicts shipped behavior; multi-signal inference where a recorded fact belongs; a new default-off capability with no named enablement path.
|
||||
- Before landing any PR: read the latest ClawSweeper comment and its `Rank-up moves:` list. Apply each move, or state in the PR why it is skipped; never merge past them silently. No `@clawsweeper re-review` round-trip is required — the moves are already in the existing comment; re-review only refreshes the rating.
|
||||
- Changelog findings: see Docs / Changelog.
|
||||
- Public ClawSweeper comments prefer `https://docs.openclaw.ai/...` when a public docs page exists; structured evidence still cites repo files, lines, SHAs.
|
||||
- Findings need current source, shipped/current behavior, tests/CI evidence, and dependency contract proof when dependency-backed behavior is involved. Validation is judged against touched and sibling surfaces plus this file's commands; clear evidence matters for user-visible changes, with Telegram/Desktop proof for Telegram-visible behavior when feasible.
|
||||
@@ -121,10 +136,7 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- Install: `pnpm install` (keep Bun lock/patches aligned if touched). Agent dependency installation for tests/builds defaults to the selected remote box; do not reconcile a local Codex worktree just to run validation.
|
||||
- CLI: `pnpm openclaw ...` or `pnpm dev`; build: `pnpm build`.
|
||||
- Packaged `pnpm build` omits QA Lab + qa-channel by design (source-checkout only). To exercise `openclaw qa`/qa-channel from a built dist, build with `OPENCLAW_BUILD_PRIVATE_QA=1 pnpm build` (emits `dist/plugin-sdk/qa-lab.js`, `qa-runtime.js`, `dist/extensions/{qa-lab,qa-channel}`) or run via `pnpm dev`.
|
||||
- Agent proof routing: source trust first, proof size second. Only trusted source may run locally; then run one/few focused tests and cheap static checks locally when the existing checkout dependencies are ready. Use Crabbox only for larger suites, changed gates with typecheck/lint fan-out, builds, Docker, packaging, E2E, live proof, and cross-platform work. Trusted maintainer heavy proof defaults to Blacksmith Testbox. Contributor/fork code remains untrusted unless a maintainer explicitly approves credentialed execution after review; an explicit owner/maintainer instruction to land named, reviewed PRs is that approval, so do not ask twice. Otherwise use secretless fork CI or sanitized direct AWS Crabbox, never a credential-hydrated Testbox. Never run an untrusted checkout's scripts, tests, checks, wrappers, config, or package hooks locally, regardless of proof size. Sanitized AWS must launch an installed trusted Crabbox binary from a clean trusted `main` checkout and fetch only the remote PR via `--fresh-pr`; never execute a wrapper, config, or command from the untrusted local checkout. Before warmup, unset `CRABBOX_AWS_INSTANCE_PROFILE` and all `CRABBOX_TAILSCALE*` overrides; fail closed unless resolved `aws.instanceProfile` is empty. Force `--network public --tailscale=false`, clear exit-node/LAN flags, and require `crabbox inspect` to report public networking with no Tailscale state before any script. Upload trusted `scripts/crabbox-untrusted-bootstrap.sh` from clean `main` alongside `--fresh-pr`; it proves the remote IMDSv2 IAM credentials endpoint returns 404, verifies the reviewed head SHA, unsets `NODE_OPTIONS`, installs pinned Node/pnpm, verifies the package-manager pin, isolates `HOME`, installs dependencies, then runs the requested test. Use a newly warmed lease bound to one reviewed head SHA, set `CRABBOX_ENV_ALLOW=CI`, and use `--no-hydrate`. Never reuse a trusted/previously hydrated lease or carry an untrusted lease across head revisions; stop and rewarm when the SHA changes. No repo `OPENCLAW_*` allowlist, existing auth profile, instance role, tailnet/LAN access, moving PR head, or ambient Node preload may reach untrusted execution. Do not pre-warm at task start. Acquire the safe backend lazily for the first heavy proof, reuse that one lease, then stop it before handoff. Remote boxes are preferred, not mandatory: when the remote backend is unavailable (broker/DNS/network/lease failure), trusted-source work falls back to local execution — including heavier suites and gates — instead of blocking; note the fallback and reason in the proof summary. Untrusted source never falls back to local.
|
||||
- Test commands: trusted-source focused local proof uses `node scripts/run-vitest.mjs <path-or-filter>`; remote or normal-checkout proof may use `pnpm test <path-or-filter> [vitest args...]`, `pnpm test:changed`, `pnpm test:serial`, or `pnpm test:coverage`. Never raw `vitest`.
|
||||
- If raw Vitest is unavoidable, use `vitest run ...`; bare `vitest ...` starts local watch mode and will not exit on its own.
|
||||
- Vitest repetition: no `--repeat`; use a bounded shell loop around the focused repo test command.
|
||||
- Test commands: trusted-source focused local proof uses `node scripts/run-vitest.mjs <path-or-filter>`; remote or normal-checkout proof may use `pnpm test <path-or-filter> [vitest args...]`, `pnpm test:changed`, `pnpm test:serial`, or `pnpm test:coverage`. Never raw `vitest`; if unavoidable, `vitest run ...` (bare `vitest` starts local watch mode and never exits). No `--repeat`; use a bounded shell loop around the focused repo test command.
|
||||
- Local agent test execution is allowed only for trusted source and one/few focused files when the existing dependency install is ready. In a Codex worktree or linked/sparse checkout, use `node scripts/run-vitest.mjs <path-or-filter>`; never direct local `pnpm test*`, and never reconcile dependencies merely to keep proof local.
|
||||
- Checks/lint in a trusted normal source checkout: `pnpm check:changed` classifies first; docs-only, no-change, and small metadata plans stay local when dependencies are ready, while typecheck/lint fan-out delegates to Crabbox/Testbox. Never run this repository-controlled classifier locally for untrusted source. Inspect lanes with `pnpm changed:lanes --json`; staged/path-scoped forms are `pnpm check:changed --staged` and `pnpm check:changed -- <files...>`.
|
||||
- Checks in a trusted Codex worktree or linked/sparse checkout: avoid direct local `pnpm check*`; use `node scripts/check-changed.mjs [--staged|-- <files...>]`. It can classify without installed dependencies and delegates heavy or dependency-missing proof before loading package-backed helpers. For untrusted source, do not execute this repository-controlled wrapper locally.
|
||||
@@ -140,40 +152,21 @@ Skills own workflows; root owns hard policy and routing.
|
||||
## Validation
|
||||
|
||||
- Use `$openclaw-testing` for test/CI choice and `$crabbox` for remote/full/E2E proof.
|
||||
- Classify source trust before proof size. Do not pre-warm for anticipated work. Run focused proof locally only for trusted source; untrusted source uses secretless fork CI or sanitized direct AWS regardless of size. Lazily acquire the appropriate backend at the first remote command. Trusted maintainer heavy proof prefers Blacksmith Testbox; if the remote backend is unavailable, run the proof locally for trusted source and say so in the proof summary rather than blocking. Reuse one acquired lease for later heavy commands, sync the current checkout for every run, then stop it before handoff.
|
||||
- Warm Testbox from the task checkout; ownership is checkout-path scoped; `--reclaim` only for intentional transfer.
|
||||
- One Testbox lease, one active command; never sync/reclaim during a run.
|
||||
- Testbox `--reclaim` does not retarget the remote checkout; never cross repos.
|
||||
- Base/head changed: stop and rewarm Testbox; never override stale lease checks.
|
||||
- Compound Testbox commands: `bash -lc`, never `sh -lc`; job env uses Bash `declare`.
|
||||
- Testbox cleanup: `blacksmith testbox stop --id <tbx_id>`; id is not positional.
|
||||
- Delegated Testbox rejects `--fresh-pr` and `--stop-after`; sync current checkout, workflow owns lifecycle.
|
||||
- PR review artifacts: keep template enum values; put evidence detail in summaries.
|
||||
- Crabbox request means real scenario proof: install/update/call/repro user path; not just copy tests and run them remotely.
|
||||
- Blacksmith Testbox delegated runs: omit `--stop-after`; unsupported, cleanup is delegated.
|
||||
- Proof routing: source trust first, proof size second. Trusted source runs one/few focused tests, `git diff --check`, targeted formatting, and cheap static probes locally when the existing dependency install is ready. Heavy proof — full suites, changed gates with typecheck/lint fan-out, builds, Docker, packaging, E2E, live, cross-OS, or anything computationally intensive — goes to Crabbox/Testbox; trusted maintainer heavy proof defaults to Blacksmith Testbox. If the remote backend is unavailable (broker/DNS/network/lease failure), trusted-source proof falls back to local execution — including heavier suites and gates — instead of blocking; note the fallback and reason in the proof summary.
|
||||
- Untrusted (contributor/fork) source: never run its scripts, tests, checks, wrappers, config, or package hooks locally, regardless of proof size, and never fall back to local. Use secretless fork CI or sanitized direct AWS Crabbox, never a credential-hydrated Testbox. Maintainer approval of credentialed execution after review makes it trusted; an explicit owner/maintainer instruction to land named, reviewed PRs is that approval — do not ask twice.
|
||||
- Sanitized AWS Crabbox procedure: launch an installed trusted Crabbox binary from a clean trusted `main` checkout; fetch only the remote PR via `--fresh-pr`; never execute a wrapper, config, or command from the untrusted local checkout. Upload trusted `scripts/crabbox-untrusted-bootstrap.sh` from clean `main` — it proves the remote IMDSv2 IAM credentials endpoint returns 404, verifies the reviewed head SHA, unsets `NODE_OPTIONS`, installs pinned Node/pnpm, verifies the package-manager pin, isolates `HOME`, installs dependencies, then runs the requested test. Before warmup: unset `CRABBOX_AWS_INSTANCE_PROFILE` and all `CRABBOX_TAILSCALE*` overrides; fail closed unless resolved `aws.instanceProfile` is empty; force `--network public --tailscale=false`, clear exit-node/LAN flags; require `crabbox inspect` to report public networking with no Tailscale state before any script. Use a newly warmed lease bound to one reviewed head SHA with `CRABBOX_ENV_ALLOW=CI` and `--no-hydrate`; never reuse a trusted/previously hydrated lease or carry an untrusted lease across head revisions — stop and rewarm when the SHA changes. No repo `OPENCLAW_*` allowlist, existing auth profile, instance role, tailnet/LAN access, moving PR head, or ambient Node preload may reach untrusted execution.
|
||||
- Leases: do not pre-warm at task start. Acquire the backend lazily at the first heavy proof, reuse that one lease (sync the current checkout for every run), then stop it before handoff. If local proof fans out or becomes expensive, stop it and acquire the remote box.
|
||||
- Testbox mechanics: warm from the task checkout; ownership is checkout-path scoped; `--reclaim` only for intentional transfer, and it does not retarget the remote checkout — never cross repos. One lease, one active command; never sync/reclaim during a run; base/head changed means stop and rewarm — never override stale lease checks. Warmup must print a lease id; silent success is unusable — verify before reuse, else fall back to one-shot `run`. Wrapper reuse requires its local SSH key; missing after restart/handoff means warm fresh. Direct lease: `blacksmith testbox run`; Crabbox wrapper reuse needs a wrapper-created lease. Status/stop: `blacksmith testbox status|stop --id <tbx_id>` — id is not positional, no status `--json` flag. Delegated runs reject `--fresh-pr` and `--stop-after`; sync current checkout, workflow owns lifecycle. Compound commands: `bash -lc`, never `sh -lc`; job env uses Bash `declare`. Testbox owns Chromium; never pass Crabbox `--browser` to `provider=blacksmith-testbox`.
|
||||
- Crabbox mechanics: a Crabbox request means real scenario proof — install/update/call/repro the user path, not just copied tests run remotely. Final timing JSON = proof complete; if portal sync hangs after it, interrupt the wrapper only. Wrapper `stop` has no `--timing-json`; use `node scripts/crabbox-wrapper.mjs stop --provider <provider> --id <id>`. Sparse-sync temp checkout may claim a kept Testbox; repo-path reuse needs `--reclaim`. Dirty-sync generator proof: compare hashes before/after; `git diff` includes the synced patch.
|
||||
- Visual proof: use Crabbox, set up like a user, then screenshot-verify. No harness/bypass/shortcut unless explicitly asked.
|
||||
- Trusted-source local agent work includes one/few focused tests, `git diff --check`, targeted formatting, and cheap static probes when dependencies already exist. Untrusted source executes none of its repository-controlled tooling locally. Computationally intensive work uses the selected remote box.
|
||||
- In Codex or linked worktrees, direct local `pnpm test*`, `pnpm check*`, `pnpm crabbox:run`, and `scripts/committer` can trigger pnpm dependency reconciliation or install prompts. Prefer `node` wrappers locally and Crabbox/Testbox for pnpm-gated proof.
|
||||
- Direct Blacksmith lease: use `blacksmith testbox run`; Crabbox wrapper reuse needs a wrapper-created lease.
|
||||
- Wrapper Testbox reuse requires its local SSH key; missing after restart/handoff means warm fresh.
|
||||
- Dirty-sync generator proof: compare hashes before/after; `git diff` includes the synced patch.
|
||||
- Crabbox wrapper `stop` has no `--timing-json`; use `node scripts/crabbox-wrapper.mjs stop --provider <provider> --id <id>`.
|
||||
- Repo-native PR worktree may omit `node_modules`; prove remotely, then use `git commit --no-verify`, not `scripts/committer`.
|
||||
- Release-branch formatting: Testbox or existing binary; never local `pnpm exec` reconciliation.
|
||||
- Targeted local format/lint: existing `./node_modules/.bin/*`; never `pnpm exec` reconciliation.
|
||||
- Release-branch formatting: Testbox or existing binary; never local `pnpm exec` reconciliation. Targeted local format/lint: existing `./node_modules/.bin/*`; never `pnpm exec` reconciliation.
|
||||
- Parallel agents share the checkout; never switch its branch while sibling work runs.
|
||||
- Testbox status: `blacksmith testbox status --id <tbx_id>`; no `--json` flag.
|
||||
- QA CLI `--output-dir` must be repo-relative.
|
||||
- Full suites, changed gates, builds, typechecks, lint fan-out, Docker/package/E2E/live/cross-OS proof, or anything computationally intensive: Crabbox/Testbox.
|
||||
- Testbox owns Chromium; never pass Crabbox `--browser` to `provider=blacksmith-testbox`.
|
||||
- Testbox warmup must print a lease id; silent success is unusable. Verify before reuse; fall back to one-shot `run`.
|
||||
- If local proof fans out or becomes expensive, stop it and lazily acquire the remote box.
|
||||
- Before handoff/push: prove touched surface. Before landing to `main`: proof matches actual risk. Bounded behavior-neutral refactor: focused tests/checks enough; no issue proof or full/broad suite by default.
|
||||
- Release-branch full validation: freeze the product-complete **Code SHA**, then use `node scripts/full-release-validation-at-sha.mjs --sha <code-sha> --target-ref release/YYYY.M.PATCH`; no raw dispatch without `target_context_ref`.
|
||||
- Pre-land/pre-commit code changes: mandatory fresh `$autoreview` until no accepted/actionable findings remain. Do not land code on CI, ClawSweeper, prior review comments, or your own manual review alone unless user explicitly opts out or scope is truly trivial/docs-only. If findings want refactor, refactor; no ugly fixes.
|
||||
- Before landing any PR: read the latest ClawSweeper comment and its `Rank-up moves:` list. Apply each move, or state in the PR why it is skipped; never merge past them silently. No `@clawsweeper re-review` round-trip is required — the moves are already in the existing comment; re-review only refreshes the rating.
|
||||
- Autoreview uncommitted changes: `--mode uncommitted`; no `dirty` mode.
|
||||
- Autoreview staged/uncommitted diff: use `--mode uncommitted`; no `staged` mode.
|
||||
- Pre-land/pre-commit code changes: mandatory fresh `$autoreview` until no accepted/actionable findings remain. Do not land code on CI, ClawSweeper, prior review comments, or your own manual review alone unless user explicitly opts out or scope is truly trivial/docs-only. If findings want refactor, refactor; no ugly fixes. Autoreview staged/uncommitted diff: `--mode uncommitted`; there is no `dirty` or `staged` mode.
|
||||
- If proof is blocked, say exactly what is missing and why.
|
||||
- Do not land related failing format/lint/type/build/tests. If unrelated on latest `origin/main`, say so with scoped proof.
|
||||
- Landing PR onto red `main` (unrelated breakage blocks the merge gate): fix the breakage in the same landing PR; note it in the PR body; never land onto red or bypass the gate. Prefer the smallest correct fix (e.g. register a missing source file, add a dropped export).
|
||||
@@ -188,41 +181,6 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- Use `$openclaw-pr-maintainer` immediately for maintainer-side OpenClaw issue/PR review, triage, duplicates, labels, comments, close, land, or evidence. Contributor PR creation/refresh follows the requested contributor workflow; linked refs alone do not require maintainer archive tooling.
|
||||
- Issue/PR start: `git status -sb`; if clean, `git pull --ff-only`; if dirty, yell before pull/rebase.
|
||||
- PR refs: `gh pr view/diff` or `gh api`, not web search. Prefer `gitcrawl` for maintainer discovery; missing/stale `gitcrawl` falls through to live `gh`, not contributor setup. Verify live with `gh` before mutation.
|
||||
- `gh pr view` takes the branch positionally; no `--head` flag.
|
||||
- `gh pr diff` has no `--stat`; use `gh pr view --json changedFiles,additions,deletions` or `git diff --stat`.
|
||||
- zsh: quote `gh api` endpoints containing `?` or brackets; otherwise glob expansion corrupts the invocation.
|
||||
- `gh pr checks --json`: use `link`, not nonexistent `detailsUrl`.
|
||||
- Blacksmith Testbox status/stop: `--id <tbx_id>`; no status JSON flag.
|
||||
- Crabbox final timing JSON = proof complete; if portal sync hangs after it, interrupt wrapper only.
|
||||
- Sparse-sync temp checkout may claim kept Testbox; repo-path reuse needs `--reclaim`.
|
||||
- GitHub Actions: resolve workflow files from `.github/workflows` or API; never infer filenames from display names.
|
||||
- Yielded exec: retain the returned session id before polling; never blind-retry.
|
||||
- zsh: quote command globs; unmatched patterns abort before the tool runs.
|
||||
- Nested remote shell: avoid local `$()` expansion; use remote-safe validation.
|
||||
- zsh: don't use `path` as a variable; it rewrites `$PATH`.
|
||||
- File lists into tools: `--name-only -z | xargs -0`; zsh scalars don't word-split, and a zero-file run exits 0 looking clean.
|
||||
- `scripts/pr` artifacts: preserve template enum values; validate before prepare.
|
||||
- `scripts/pr` subcommands require a PR number; no subcommand `--help` placeholder.
|
||||
- `scripts/pr` review: checkout main baseline, then PR, before artifact validation.
|
||||
- Review artifacts: validate from PR-head mode; moving main invalidates main-baseline guard.
|
||||
- `scripts/pr` prepare/merge: `main` PRs only; non-main uses reviewed release-branch flow.
|
||||
- After every PR push, rerun `scripts/pr review-init`; checkout alone leaves stale guard SHA.
|
||||
- `rg`: options/globs before `--`; `--` immediately before a leading-dash pattern only.
|
||||
- `gh --jq` is not standalone `jq`; pipe JSON to `jq` for variables or `--arg`.
|
||||
- Shared checkout: serialize `git fetch`; on ref-lock failure, re-read the ref before retry.
|
||||
- Main locked elsewhere: detach at `origin/main`, then create the task branch.
|
||||
- Git fetch/pull yielding without completion: inspect/stop only the owned process before retry; never overlap retries.
|
||||
- `gh api --paginate '<endpoint>' | jq -s ...`; gh `--slurp` may emit nothing and forbids `--jq`/`--template`.
|
||||
- Main-bound workflow dispatch: resolve server `main` SHA immediately before dispatch; retry if identity fails after `main` advances.
|
||||
- `release-ci-summary` accepts Full Release Validation parent runs only.
|
||||
- Diverged release-branch logs: use `--first-parent` plus a bounded count.
|
||||
- `gh run view --json` uses `attempt`, not `attemptNumber`.
|
||||
- GH job logs: filter the exact tab-delimited step first; broad patterns also match the job name.
|
||||
- Path formatter: `node_modules/.bin/oxfmt`; `pnpm exec` may reconcile workspace deps.
|
||||
- Crabbox stop: no `--timing-json`; use `node scripts/crabbox-wrapper.mjs stop --provider <provider> --id <id>`.
|
||||
- macOS `find` has no `-printf`; use `-print0` plus `stat`.
|
||||
- Actions checkout refs: use full 40-char SHAs; short SHAs resolve as branches/tags.
|
||||
- zsh Git object paths: use `${sha}:path`; `$sha:path` invokes parameter modifiers.
|
||||
- Bare issue/PR URL/number: inspect live and take the efficient maintainer path; switch branches/refs when useful.
|
||||
- No unsolicited PR labels/retitles/rebases/fixups/landing. Comments/reviews ok only for reviewable findings, pre-merge proof, or close/duplicate reason after explicit close/sweep/landing request.
|
||||
- Maintainer decision closes the cluster: if deciding reported behavior/proposed fix is not planned, comment+close all directly associated open issues/PRs unless explicitly told to keep one open. Associated means linked PRs/issues, duplicates, companion workaround PRs, and the canonical issue for the rejected behavior.
|
||||
@@ -250,16 +208,28 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- GitHub issue/PR create: read `$agent-transcript`; ask about sanitized transcript logs when available.
|
||||
- Contributor PRs: parsed context requires authored `What Problem This Solves` and `Evidence` sections. Do not require field-level proof forms; reviewers inspect code, tests, and CI for correctness.
|
||||
- PR artifacts/screenshots: attach to PR/comment/external artifact store. Never push screenshots, videos, proof images, or proof assets to OpenClaw or any product repo branch, including temp artifact branches. Use Crabbox artifact publishing plus the manifest URL. Do not commit `.github/pr-assets`.
|
||||
- CI polling: exact SHA, relevant checks only, minimal fields. Skip routine noise (`Auto response`, `Labeler`, docs agents, performance/stale). Logs only after failure/completion or concrete need. Never `gh run watch`; its 3s polling exhausts API quota. Use sparse GraphQL rollups. Filter `gh run list` by workflow/branch/commit; broad JSON lists can exceed relay caps. `gh --jq` has no jq `--arg`; use `--commit` for SHA filtering. Reruns need `gh run view <run> --attempt <n>`; default output may show the prior attempt. Exact-SHA fallback dispatches require the full 40-character SHA.
|
||||
- CI waits: node scripts/watch-pr-ci.mjs <pr> <head-sha> — prechecks mergeable (CONFLICTING = pull_request CI cannot attach) and run attachment before polling; watchers emit every terminal state; no unbounded polls.
|
||||
- CI polling: exact SHA, relevant checks only, minimal fields. Skip routine noise (`Auto response`, `Labeler`, docs agents, performance/stale). Logs only after failure/completion or concrete need. Never `gh run watch`; its 3s polling exhausts API quota. Use sparse GraphQL rollups. Filter `gh run list` by workflow/branch/commit; broad JSON lists can exceed relay caps. Exact-SHA fallback dispatches require the full 40-character SHA.
|
||||
- CI waits: `node scripts/watch-pr-ci.mjs <pr> <head-sha>` — prechecks mergeable (CONFLICTING = pull_request CI cannot attach) and run attachment before polling; watchers emit every terminal state; no unbounded polls.
|
||||
- Trusted-workflow release-branch CI: pass `target_ref` + `release_candidate_ref`; never `release_gate` (requires workflow head == target).
|
||||
- Agent PR landing to `main`: use only the repo-native `scripts/pr` wrapper: run `scripts/pr review-init <PR>`, follow its emitted checkout/guard guidance, initialize and complete review artifacts with `scripts/pr review-artifacts-init <PR>`, validate them with `scripts/pr review-validate-artifacts <PR>`, then run `OPENCLAW_TESTBOX=1 scripts/pr prepare-run <PR>` and `scripts/pr merge-run <PR>`. The Testbox flag is mandatory for agents so prepare verifies hosted CI/Testbox on the current head or reuses a patch-identical pre-rebase run green within 24 hours instead of running full gates locally. `prepare-run` fails fast; invoke only after exact-head CI is complete and green. For owner-approved reviewed fork code without hosted Testbox, use `OPENCLAW_PR_GATES_REMOTE=testbox` instead. Do not rebase only because `main` advanced; merge drift is advisory unless strict drift is explicitly enabled, while GitHub still blocks conflicts. Do not idle on `auto-response` or `check-docs`.
|
||||
- After GitHub throttling, check core quota before `scripts/pr prepare-run` or `merge-run`. A failed operation can retain its lock; verify no child remains, then recover only with its emitted token.
|
||||
- Local `scripts/pr`: unset `GITHUB_TOKEN`, `GH_TOKEN`, `HOMEBREW_GITHUB_API_TOKEN`; ambient tokens can select an exhausted or wrong identity.
|
||||
- Non-main PRs: do not run `scripts/pr prepare-run` or `merge-run`; they diff against `main`. Use review artifacts, exact base-head CI, revalidate `headRefOid`, then `gh pr merge --match-head-commit <verified-sha>`.
|
||||
- Merge guard shells: start `set -euo pipefail`; a failed `[[ ... ]]` alone does not stop a later merge command.
|
||||
- After `scripts/pr merge-run` removes its worktree, `cd` to a persistent repo before follow-up commands.
|
||||
- `scripts/pr` review JSON: land-ready recommendation `READY FOR /prepare-pr`, `issueValidation.status=valid`; never `APPROVE`.
|
||||
- Main-bound workflow dispatch: resolve server `main` SHA immediately before dispatch; retry if identity fails after `main` advances.
|
||||
- `release-ci-summary` accepts Full Release Validation parent runs only. Diverged release-branch logs: `--first-parent` plus a bounded count.
|
||||
|
||||
## Tooling Gotchas
|
||||
|
||||
Mechanics only; policy lives above.
|
||||
|
||||
- `gh`: `gh pr view` takes the branch positionally (no `--head`). `gh pr diff` has no `--stat`; use `gh pr view --json changedFiles,additions,deletions` or `git diff --stat`. `gh pr checks --json` uses `link`, not `detailsUrl`. `gh run view --json` uses `attempt`, not `attemptNumber`; reruns need `gh run view <run> --attempt <n>` (default output may show the prior attempt). `gh --jq` is not standalone `jq` (no `--arg`); pipe JSON to `jq`. `gh api --paginate '<endpoint>' | jq -s ...`; gh `--slurp` may emit nothing and forbids `--jq`/`--template`.
|
||||
- zsh: quote `gh api` endpoints containing `?` or brackets and quote command globs; unmatched patterns abort before the tool runs. Don't use `path` as a variable; it rewrites `$PATH`. Git object paths: `${sha}:path`; `$sha:path` invokes parameter modifiers. File lists into tools: `--name-only -z | xargs -0`; zsh scalars don't word-split, and a zero-file run exits 0 looking clean.
|
||||
- git: shared checkout — serialize `git fetch`; on ref-lock failure, re-read the ref before retry. Fetch/pull yielding without completion: inspect/stop only the owned process before retry; never overlap retries. Main locked elsewhere: detach at `origin/main`, then create the task branch.
|
||||
- GitHub Actions: resolve workflow files from `.github/workflows` or API; never infer filenames from display names. Checkout refs use full 40-char SHAs; short SHAs resolve as branches/tags. GH job logs: filter the exact tab-delimited step first; broad patterns also match the job name.
|
||||
- Shell/exec: yielded exec — retain the returned session id before polling; never blind-retry. Nested remote shell: avoid local `$()` expansion; use remote-safe validation. Merge guard shells start `set -euo pipefail`; a failed `[[ ... ]]` alone does not stop a later merge command.
|
||||
- `rg`: options/globs before `--`; `--` immediately before a leading-dash pattern only.
|
||||
- macOS `find` has no `-printf`; use `-print0` plus `stat`.
|
||||
- Path formatter: `node_modules/.bin/oxfmt`; `pnpm exec` may reconcile workspace deps.
|
||||
- `scripts/pr`: subcommands require a PR number (no subcommand `--help` placeholder). Artifacts preserve template enum values with evidence detail in summaries; validate before prepare, from PR-head mode (moving main invalidates the main-baseline guard). Review flow: checkout main baseline, then PR, before artifact validation. After every PR push, rerun `scripts/pr review-init`; checkout alone leaves a stale guard SHA. Locally unset `GITHUB_TOKEN`, `GH_TOKEN`, `HOMEBREW_GITHUB_API_TOKEN`; ambient tokens can select an exhausted or wrong identity. Review JSON: land-ready recommendation `READY FOR /prepare-pr`, `issueValidation.status=valid`; never `APPROVE`. After `scripts/pr merge-run` removes its worktree, `cd` to a persistent repo before follow-up commands.
|
||||
|
||||
## Code
|
||||
|
||||
@@ -275,8 +245,7 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- Correct but not over-engineered. Correctness on real inputs/states is mandatory; extra layers, guards, and generality for imagined ones are defects, not rigor.
|
||||
- Codebase is already large; pragmatism wins. Extremely unlikely edge cases are tradable for real simplification — name the accepted tradeoff (comment or PR) so it is a decision, not an oversight.
|
||||
- Code size matters. Prefer small clear code; maintainability includes not growing LOC without payoff.
|
||||
- Refactors should delete about as much local complexity as they add. If LOC grows, the new ownership/API needs to clearly pay for it.
|
||||
- Refactors should reduce non-test LOC unless they remove a larger architectural cost. Treat positive prod LOC as a smell. Before closeout, run `git diff --numstat`; if non-test LOC grew, trim or explicitly justify why fewer paths now exist.
|
||||
- Refactors should delete about as much local complexity as they add, and reduce non-test LOC unless they remove a larger architectural cost. Treat positive prod LOC as a smell. Before closeout, run `git diff --numstat`; if non-test LOC grew, trim or explicitly justify why fewer paths now exist.
|
||||
- Prefer deleting branches, modes, adapters, and tests over preserving them. A refactor that adds a second path has probably failed unless the old path is a cited shipped contract.
|
||||
- New helpers/files must pay rent immediately: fewer call paths, fewer concepts, or less repeated logic. No helpers for one-off compat, naming translation, or speculative resilience.
|
||||
- Before adding helpers/files, check whether existing code can absorb the behavior with less new surface.
|
||||
@@ -287,7 +256,6 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- Tests prove behavior/regressions, not every internal branch.
|
||||
- Tests are welcome, but review them before landing for duplication and value. Delete useless tests, such as assertions for behavior or paths just removed.
|
||||
- Tests protect canonical behavior and migration boundaries, not obsolete internals. Delete tests for removed fallback paths instead of updating them.
|
||||
- For non-trivial refactors, check `git diff --numstat` before closeout. If LOC grew, trim or explain why.
|
||||
- Prefer existing narrow helpers over repeated casts/guards. Add local helpers when 2+ nearby call sites share real boundary logic.
|
||||
- Prefer ctor parameter properties for injected deps/config. Do not ban them for erasable-syntax purity.
|
||||
- Prefer `satisfies` for registries/config maps; derive types from schemas when a runtime schema already exists.
|
||||
@@ -305,6 +273,9 @@ Skills own workflows; root owns hard policy and routing.
|
||||
## Tests
|
||||
|
||||
- Vitest. Colocated `*.test.ts`; e2e `*.e2e.test.ts`; example models `sonnet-4.6`, `gpt-5.6-luna`; test GPT with Luna preferred; use Sol when capability matters; no GPT-4.x agent-smoke defaults.
|
||||
- Test where the bugs live: boundaries, not internals. Coverage behind mocks proves the mocks; one test through the real transport/dispatch seam outranks many stub-backed branch tests.
|
||||
- Prefer invariant assertions (every input accounted for; every action ends in a visible outcome or recorded non-outcome) over enumerating happy paths.
|
||||
- Inject faults — network, provider, ordering, restart — instead of asserting only success shapes. Changes to delivery, dispatch, or session paths need at least one boundary-level proof (harness or live), not only unit tests of the changed function.
|
||||
- Prefer behavior tests over workflow/docs string greps. Put operator policy reminders in AGENTS/docs.
|
||||
- QA scenario sources are YAML only: `qa/scenarios/index.yaml` and `qa/scenarios/<theme>/*.yaml`. Do not add fenced `qa-scenario`/`qa-flow` Markdown files under `qa/scenarios/`.
|
||||
- Clean timers/env/globals/mocks/sockets/temp dirs/module state; `--isolate=false` safe.
|
||||
@@ -350,13 +321,8 @@ Skills own workflows; root owns hard policy and routing.
|
||||
- `pnpm-lock.yaml` is the product dependency security review surface; `.github/release/clawhub-cli/package-lock.json` separately pins trusted release tooling. Published packages bundle runtime dependencies where configured and never ship lockfiles; other npm-format locks exist only transiently during checks and publish staging.
|
||||
- Carbon pins owner-only: do not change `@buape/carbon` unless Shadow (`@thewilloftheshadow`, verified by `gh`) asks.
|
||||
- Releases/publish/version bumps need explicit approval. Use `$release-openclaw-maintainer`.
|
||||
- Active release scope lock: freeze the operator-selected cut SHA and release
|
||||
identity through publish and verification. Moving `main`, unrelated CI,
|
||||
optional backports, refactors, cleanup, and normal forward-ports are not part
|
||||
of the release work queue.
|
||||
- Touch `main` during a release only when the operator requests it or the
|
||||
smallest critical main-owned blocker prevents that release. Return to the
|
||||
release branch immediately; defer broader main work until closeout.
|
||||
- Active release scope lock: freeze the operator-selected cut SHA and release identity through publish and verification. Moving `main`, unrelated CI, optional backports, refactors, cleanup, and normal forward-ports are not part of the release work queue.
|
||||
- Touch `main` during a release only when the operator requests it or the smallest critical main-owned blocker prevents that release. Return to the release branch immediately; defer broader main work until closeout.
|
||||
- Release versions use `YYYY.M.PATCH`, where `PATCH` is a sequential monthly release-train number, never the calendar day. Stable and beta tags determine the current train; alpha-only tags do not consume or advance the beta/stable patch number. After `2026.6.5`, the next beta train is `2026.6.6-beta.1` even if higher alpha-only tags exist.
|
||||
- Alpha/nightly versions use the next unreleased train plus an incrementing prerelease number. Repeated nightlies for the same train increment only `alpha.N`; they must not mint a new patch number from the date.
|
||||
- Backports are optional. Apply only the operator-selected set; when requested without a target, use the newest open `release/` branch.
|
||||
|
||||
Reference in New Issue
Block a user