diff --git a/AGENTS.md b/AGENTS.md index 6e3447bff33..504bdd8bd2a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,7 +61,7 @@ Scoped guides: - Build: `pnpm build` - Smart local gate: `pnpm check:changed` (scoped typecheck/lint/guards + relevant tests) - Explain smart gate: `pnpm changed:lanes --json` -- Pre-commit view: `pnpm check:changed --staged` +- Staged gate preview: `pnpm check:changed --staged` - Normal full prod sweep: `pnpm check` (prod typecheck/lint/guards, no tests) - Full tests: `pnpm test` - Changed tests only: `pnpm test:changed` @@ -99,7 +99,7 @@ Scoped guides: ## Gates -- Pre-commit hook: staged format/lint, then `pnpm check:changed --staged`; docs/markdown-only skips changed-scope check; `FAST_COMMIT=1` / `scripts/committer --fast` skips changed-scope check only. +- Pre-commit hook: staged formatting only. It does not run lint, typecheck, or tests. - Changed lanes: - core prod => core prod typecheck + core tests - core tests => core test typecheck/tests only @@ -107,11 +107,11 @@ Scoped guides: - extension tests => extension test typecheck/tests only - public SDK/plugin contract => extension prod/test validation too - unknown root/config => all lanes -- Local loop: prefer `pnpm check:changed`; use `pnpm test:changed` for tests only; use `pnpm check` for full prod TS/lint sweep without tests. +- Local loop: run `pnpm check:changed` explicitly before handoff/push; use `pnpm test:changed` for tests only; use `pnpm check` for full prod TS/lint sweep without tests. - Landing on `main`: verify touched surface near landing; default bar is `pnpm check` + `pnpm test` when feasible. - Hard build gate: run/pass `pnpm build` before push if build output, packaging, lazy/module boundaries, or published surfaces can change. - Do not land related failing format/lint/type/build/tests. If failures are unrelated on latest `origin/main`, say so and give scoped proof. -- Fast commit escape hatch: use `scripts/committer --fast "" ` only after the exact staged change set was already validated with equal-or-stronger gates, or after rerunning an isolated flaky failure with proof. State the gates/proof in handoff. +- Commit helper is formatting-only; validation gates are explicit commands, not commit side effects. - CI architecture gate: `check-additional`; local equivalent `pnpm check:architecture`. - Config docs drift: `pnpm config:docs:gen/check` - Plugin SDK API drift: `pnpm plugin-sdk:api:gen/check` @@ -162,7 +162,7 @@ Scoped guides: ## Git -- Use `scripts/committer "" `; stage only intended files. Use `--fast` only under the Gates escape-hatch rule above. +- Use `scripts/committer "" `; stage only intended files. It formats staged files only; run validation separately. - Commits: conventional-ish, concise/action-oriented. Group related changes. - No manual stash/autostash unless explicitly requested. No branch/worktree changes unless requested. - No merge commits on `main`; rebase on latest `origin/main` before push. diff --git a/docs/help/testing.md b/docs/help/testing.md index 3cad23d7ecc..fb27fc70c4b 100644 --- a/docs/help/testing.md +++ b/docs/help/testing.md @@ -315,7 +315,7 @@ Think of the suites as “increasing realism” (and increasing flakiness/cost): - No real keys required - Should be fast and stable - - Untargeted `pnpm test` runs twelve smaller shard configs (`core-unit-fast`, `core-unit-src`, `core-unit-security`, `core-unit-ui`, `core-unit-support`, `core-support-boundary`, `core-contracts`, `core-bundled`, `core-runtime`, `agentic`, `auto-reply`, `extensions`) instead of one giant native root-project process. This cuts peak RSS on loaded machines and avoids auto-reply/extension work starving unrelated suites. - `pnpm test --watch` still uses the native root `vitest.config.ts` project graph, because a multi-shard watch loop is not practical. - `pnpm test`, `pnpm test:watch`, and `pnpm test:perf:imports` route explicit file/directory targets through scoped lanes first, so `pnpm test extensions/discord/src/monitor/message-handler.preflight.test.ts` avoids paying the full root project startup tax. - `pnpm test:changed` expands changed git paths into the same scoped lanes when the diff only touches routable source/test files; config/setup edits still fall back to the broad root-project rerun. - `pnpm check:changed` is the normal smart local gate for narrow work. It classifies the diff into core, core tests, extensions, extension tests, apps, docs, release metadata, and tooling, then runs the matching typecheck/lint/test lanes. Public Plugin SDK and plugin-contract changes include extension validation because extensions depend on those core contracts. Release metadata-only version bumps run targeted version/config/root-dependency checks instead of the full suite, with a guard that rejects package changes outside the top-level version field. - Import-light unit tests from agents, commands, plugins, auto-reply helpers, `plugin-sdk`, and similar pure utility areas route through the `unit-fast` lane, which skips `test/setup-openclaw-runtime.ts`; stateful/runtime-heavy files stay on the existing lanes. - Selected `plugin-sdk` and `commands` helper source files also map changed-mode runs to explicit sibling tests in those light lanes, so helper edits avoid rerunning the full heavy suite for that directory. - `auto-reply` has three dedicated buckets: top-level core helpers, top-level `reply.*` integration tests, and the `src/auto-reply/reply/**` subtree. This keeps the heaviest reply harness work off the cheap status/chunk/token tests. + - Untargeted `pnpm test` runs twelve smaller shard configs (`core-unit-fast`, `core-unit-src`, `core-unit-security`, `core-unit-ui`, `core-unit-support`, `core-support-boundary`, `core-contracts`, `core-bundled`, `core-runtime`, `agentic`, `auto-reply`, `extensions`) instead of one giant native root-project process. This cuts peak RSS on loaded machines and avoids auto-reply/extension work starving unrelated suites. - `pnpm test --watch` still uses the native root `vitest.config.ts` project graph, because a multi-shard watch loop is not practical. - `pnpm test`, `pnpm test:watch`, and `pnpm test:perf:imports` route explicit file/directory targets through scoped lanes first, so `pnpm test extensions/discord/src/monitor/message-handler.preflight.test.ts` avoids paying the full root project startup tax. - `pnpm test:changed` expands changed git paths into the same scoped lanes when the diff only touches routable source/test files; config/setup edits still fall back to the broad root-project rerun. - `pnpm check:changed` is the normal smart local gate for narrow work. It classifies the diff into core, core tests, extensions, extension tests, apps, docs, release metadata, and tooling, then runs the matching typecheck/lint/test lanes. Public Plugin SDK and plugin-contract changes include one extension validation pass because extensions depend on those core contracts. Release metadata-only version bumps run targeted version/config/root-dependency checks instead of the full suite, with a guard that rejects package changes outside the top-level version field. - Import-light unit tests from agents, commands, plugins, auto-reply helpers, `plugin-sdk`, and similar pure utility areas route through the `unit-fast` lane, which skips `test/setup-openclaw-runtime.ts`; stateful/runtime-heavy files stay on the existing lanes. - Selected `plugin-sdk` and `commands` helper source files also map changed-mode runs to explicit sibling tests in those light lanes, so helper edits avoid rerunning the full heavy suite for that directory. - `auto-reply` has three dedicated buckets: top-level core helpers, top-level `reply.*` integration tests, and the `src/auto-reply/reply/**` subtree. This keeps the heaviest reply harness work off the cheap status/chunk/token tests. @@ -348,17 +348,11 @@ Think of the suites as “increasing realism” (and increasing flakiness/cost): - `pnpm changed:lanes` shows which architectural lanes a diff triggers. - - The pre-commit hook runs `pnpm check:changed --staged` after staged - formatting/linting, so core-only commits do not pay extension test cost - unless they touch public extension-facing contracts. Release - metadata-only commits stay on the targeted - version/config/root-dependency lane. - - If the exact staged change set was already validated with - equal-or-stronger gates, use - `scripts/committer --fast "" ` to skip only the - changed-scope hook rerun. Staged format/lint still run. Mention the - completed gates in your handoff. This is also acceptable after an - isolated flaky hook failure is rerun and passes with scoped proof. + - The pre-commit hook is formatting-only. It restages formatted files and + does not run lint, typecheck, or tests. + - Run `pnpm check:changed` explicitly before handoff or push when you + need the smart local gate. Public Plugin SDK and plugin-contract + changes include one extension validation pass. - `pnpm test:changed` routes through scoped lanes when the changed paths map cleanly to a smaller suite. - `pnpm test:max` and `pnpm test:changed:max` keep the same routing diff --git a/docs/reference/test.md b/docs/reference/test.md index 07e69211727..3901c0cfe48 100644 --- a/docs/reference/test.md +++ b/docs/reference/test.md @@ -12,7 +12,7 @@ title: "Tests" - `pnpm test:coverage:changed`: Runs unit coverage only for files changed since `origin/main`. - `pnpm test:changed`: expands changed git paths into scoped Vitest lanes when the diff only touches routable source/test files. Config/setup changes still fall back to the native root projects run so wiring edits rerun broadly when needed. - `pnpm changed:lanes`: shows the architectural lanes triggered by the diff against `origin/main`. -- `pnpm check:changed`: runs the smart changed gate for the diff against `origin/main`. It runs core work with core test lanes, extension work with extension test lanes, test-only work with test typecheck/tests only, expands public Plugin SDK or plugin-contract changes to extension validation, and keeps release metadata-only version bumps on targeted version/config/root-dependency checks. +- `pnpm check:changed`: runs the smart changed gate for the diff against `origin/main`. It runs core work with core test lanes, extension work with extension test lanes, test-only work with test typecheck/tests only, expands public Plugin SDK or plugin-contract changes to one extension validation pass, and keeps release metadata-only version bumps on targeted version/config/root-dependency checks. - `pnpm test`: routes explicit file/directory targets through scoped Vitest lanes. Untargeted runs use fixed shard groups and expand to leaf configs for local parallel execution; the extension group always expands to the per-extension shard configs instead of one giant root-project process. - Full and extension shard runs update local timing data in `.artifacts/vitest-shard-timings.json`; later runs use those timings to balance slow and fast shards. Set `OPENCLAW_TEST_PROJECTS_TIMINGS=0` to ignore the local timing artifact. - Selected `plugin-sdk` and `commands` test files now route through dedicated light lanes that keep only `test/setup.ts`, leaving runtime-heavy cases on their existing lanes. diff --git a/git-hooks/pre-commit b/git-hooks/pre-commit index 48106792874..b7a1f25ba9a 100755 --- a/git-hooks/pre-commit +++ b/git-hooks/pre-commit @@ -28,53 +28,13 @@ if [ "${#files[@]}" -eq 0 ]; then exit 0 fi -lint_files=() -while IFS= read -r -d '' file; do - lint_files+=("$file") -done < <(node "$FILTER_FILES" lint -- "${files[@]}") - format_files=() while IFS= read -r -d '' file; do format_files+=("$file") done < <(node "$FILTER_FILES" format -- "${files[@]}") -docs_only=true -for file in "${files[@]}"; do - case "$file" in - docs/*|*.md|*.mdx) - ;; - *) - docs_only=false - break - ;; - esac -done - -if [ "${#lint_files[@]}" -gt 0 ]; then - "$RUN_NODE_TOOL" oxlint --type-aware --fix -- "${lint_files[@]}" -fi - if [ "${#format_files[@]}" -gt 0 ]; then "$RUN_NODE_TOOL" oxfmt --write --no-error-on-unmatched-pattern "${format_files[@]}" fi git add -- "${files[@]}" - -# This hook is also exercised from lightweight temp repos in tests, where the -# staged-file safety behavior matters but the full OpenClaw workspace does not -# exist. Only run the repo-wide validation gate inside a real checkout. -if [[ -f "$ROOT_DIR/package.json" ]] && [[ -f "$ROOT_DIR/pnpm-lock.yaml" ]]; then - cd "$ROOT_DIR" - case "${FAST_COMMIT:-}" in - 1|true|TRUE|yes|YES|on|ON) - echo "FAST_COMMIT enabled: skipping changed-scope check in pre-commit hook." - ;; - *) - if [[ "$docs_only" == true ]]; then - echo "Docs-only staged changes detected: skipping pnpm check in pre-commit hook." - else - pnpm check:changed --staged - fi - ;; - esac -fi diff --git a/scripts/check-changed.mjs b/scripts/check-changed.mjs index 230ea9ec8dd..8802549ad0a 100644 --- a/scripts/check-changed.mjs +++ b/scripts/check-changed.mjs @@ -120,13 +120,17 @@ export function createChangedCheckPlan(result, options = {}) { } const testPlan = resolveChangedTestTargetPlan(result.paths); + const runExtensionTests = result.extensionImpactFromCore; + const testTargets = runExtensionTests + ? testPlan.targets.filter((target) => target !== "extensions") + : testPlan.targets; const runChangedTestsBroad = testPlan.mode === "broad"; return { commands, - testTargets: testPlan.targets, + testTargets, runChangedTestsBroad, runFullTests: false, - runExtensionTests: result.extensionImpactFromCore, + runExtensionTests, summary: Object.entries(lanes) .filter(([, enabled]) => enabled) .map(([lane]) => lane) diff --git a/test/git-hooks-pre-commit.test.ts b/test/git-hooks-pre-commit.test.ts index 56c57e12d81..4d9bc743b51 100644 --- a/test/git-hooks-pre-commit.test.ts +++ b/test/git-hooks-pre-commit.test.ts @@ -64,11 +64,6 @@ describe("git-hooks/pre-commit (integration)", () => { // Use the real hook script and lightweight helper stubs. const fakeBinDir = installPreCommitFixture(dir); - // The hook can end with `pnpm check:changed --staged`, but this fixture is only - // exercising staged-file handling. - // Stub pnpm too so Windows CI does not invoke a real package-manager command in the temp repo. - writeExecutable(fakeBinDir, "pnpm", "#!/usr/bin/env bash\nexit 0\n"); - // Create an untracked file that should NOT be staged by the hook. writeFileSync(path.join(dir, "secret.txt"), "do-not-stage\n", "utf8"); @@ -85,8 +80,8 @@ describe("git-hooks/pre-commit (integration)", () => { expect(staged).toEqual(["--all"]); }); - it("runs changed-scope check for non-doc staged changes", () => { - const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-check-changed-"); + it("does not run the changed-scope check for non-doc staged changes", () => { + const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-no-check-changed-"); run(dir, "git", ["init", "-q", "--initial-branch=main"]); const fakeBinDir = installPreCommitFixture(dir); @@ -95,7 +90,7 @@ describe("git-hooks/pre-commit (integration)", () => { writeExecutable( fakeBinDir, "pnpm", - "#!/usr/bin/env bash\nprintf '%s\\n' \"$*\" > pnpm-args.txt\n", + "#!/usr/bin/env bash\necho 'pnpm should not run from pre-commit' >&2\nexit 99\n", ); writeFileSync(path.join(dir, "tracked.txt"), "hello\n", "utf8"); @@ -105,11 +100,11 @@ describe("git-hooks/pre-commit (integration)", () => { PATH: `${fakeBinDir}:${process.env.PATH ?? ""}`, }); - expect(run(dir, "cat", ["pnpm-args.txt"])).toBe("check:changed --staged"); + expect(run(dir, "git", ["diff", "--cached", "--name-only"])).toBe("tracked.txt"); }); - it("skips changed-scope check when FAST_COMMIT is enabled", () => { - const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-yolo-"); + it("ignores FAST_COMMIT because the hook is already formatting-only", () => { + const dir = makeTempRepoRoot(tempDirs, "openclaw-pre-commit-fast-"); run(dir, "git", ["init", "-q", "--initial-branch=main"]); const fakeBinDir = installPreCommitFixture(dir); @@ -125,13 +120,11 @@ describe("git-hooks/pre-commit (integration)", () => { writeFileSync(path.join(dir, "tracked.txt"), "hello\n", "utf8"); run(dir, "git", ["add", "--", "tracked.txt"]); - const output = run(dir, "bash", ["git-hooks/pre-commit"], { + run(dir, "bash", ["git-hooks/pre-commit"], { PATH: `${fakeBinDir}:${process.env.PATH ?? ""}`, FAST_COMMIT: "1", }); - expect(output).toContain( - "FAST_COMMIT enabled: skipping changed-scope check in pre-commit hook.", - ); + expect(run(dir, "git", ["diff", "--cached", "--name-only"])).toBe("tracked.txt"); }); }); diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index 9c68bce392d..15b071882fd 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -165,6 +165,7 @@ describe("scripts/changed-lanes", () => { all: false, }); expect(plan.runExtensionTests).toBe(true); + expect(plan.testTargets).toEqual(["src/plugin-sdk/core.ts"]); }); it("fails safe for root config changes", () => {