Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 28 additions & 18 deletions .agents/skills/nemoclaw-contributor-create-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,25 @@ Follow the shared [Git and GitHub Access Hard Stop](../_shared/git-github-hard-s

Before creating a PR, verify the branch.

1. **Not on main.** Never create PRs from main.
1. **Refresh the trusted base ref.**

```bash
git fetch --prune origin main
```

2. **Not on main.** Never create PRs from main.

```bash
git branch --show-current
```

2. **Branch has commits ahead of main.**
3. **Branch has commits ahead of `origin/main`.**

```bash
git log main..HEAD --oneline
git log origin/main..HEAD --oneline
```

3. **Working tree is clean.** Stage or stash any uncommitted changes first.
4. **Working tree is clean.** Stage or stash any uncommitted changes first.

```bash
git status
Expand All @@ -49,29 +55,32 @@ Use the checks that match the diff and the verification you already have.

If the commits were created normally and the branch was pushed normally, count the installed hooks as verification:

- `pre-commit` runs file fixers, formatters, linters, skill frontmatter validation, and changed-surface Vitest hooks.
- `pre-commit` runs cheap structural and file-local checks, including fixers, formatters, linters, and skill frontmatter validation.
- `commit-msg` runs commitlint.
- `pre-push` runs TypeScript build and type-check gates.
- `pre-push` runs path-scoped incremental type checks for affected CLI and plugin surfaces plus checked-JavaScript checks.

If hooks were skipped with `--no-verify`, were not installed, failed, or you cannot tell whether they ran, run a manual diff-scoped fallback before creating the PR:
If hooks were skipped with `--no-verify`, were not installed, failed, or you cannot tell whether they ran, use the single diff-scoped fallback that reproduces `pre-commit`, `commit-msg`, and `pre-push` checks:

```bash
npx prek run --from-ref main --to-ref HEAD
npm run check:diff
```

Use `npx prek run --all-files` only when you need a whole-repository baseline, such as changing hook configuration, formatter configuration, generated-check scripts, or other repo-wide validation behavior.
The fallback compares with the refreshed `origin/main` ref from Step 1.
Reserve `npm run check` for the whole-repository pre-commit and full CLI/plugin coverage baseline, such as when changing hook configuration, formatter configuration, generated-check scripts, or other repo-wide validation behavior.

### Targeted Tests

Run the smallest meaningful tests for changed behavior:
Run the smallest meaningful tests for changed behavior once per relevant change set, and record the command and result for the PR body:

- CLI or root `src/`, `bin/`, `scripts/`, or `test/` changes: `npx vitest run --project cli` or the directly affected test file.
- Plugin changes under `nemoclaw/src/`: `npx vitest run --project plugin` or the directly affected plugin test file.
- E2E support changes under `test/e2e/support/`: `npx vitest run --project e2e-support`.
- E2E workflow, artifact upload, trace timing, or fixture environment-boundary changes: run the directly affected `test/e2e/support/*workflow*.test.ts`, `test/e2e/support/upload-e2e-artifacts-workflow-boundary.test.ts`, `test/e2e/support/sanitize-trace-timing.test.ts`, and fixture boundary tests instead of relying on unrelated live target runs.
- Installer behavior changes: run the relevant installer integration project only when the local environment supports it.

Reserve full `npm test` for broad runtime changes, test harness changes, or cases where targeted coverage is hard to justify.
Do not rerun targeted tests solely because the normal hooks passed; rerun them after later edits or hook autofixes that can affect the tested behavior.
Reserve `npm test` for broad runtime changes, test harness changes, or cases where targeted coverage is hard to justify.
Reserve `npm run check` for repo-wide hook, formatter, generated-check, or coverage-baseline changes.
Do not run the full test suite for doc-only changes unless the docs change code samples or generated behavior in a way that needs runtime validation.

For doc-only changes, run the docs build before opening the PR:
Expand All @@ -95,7 +104,7 @@ If the push fails because of SSH, authentication, remote access, authorization,

## Step 4: Prepare DCO Declaration and Verify GitHub Commits

Before creating the PR, prepare the DCO declaration for the PR body and verify every commit in `main..HEAD`.
Before creating the PR, prepare the DCO declaration for the PR body and verify every commit in `origin/main..HEAD`.
This is a hard contributor self-serve gate.
Do not run `gh pr create` until the PR body will include the DCO declaration and every commit passes GitHub verification.

Expand All @@ -108,10 +117,10 @@ Do not run `gh pr create` until the PR body will include the DCO declaration and
```

2. **GitHub verification.** Each pushed commit must appear as verified in GitHub.
Check the commit SHAs from `main..HEAD` with the GitHub API before opening the PR.
Check the commit SHAs from `origin/main..HEAD` with the GitHub API before opening the PR.

```bash
for sha in $(git rev-list main..HEAD); do
for sha in $(git rev-list origin/main..HEAD); do
gh api "/repos/NVIDIA/NemoClaw/commits/$sha" --jq '.sha + " verified=" + (.commit.verification.verified | tostring) + " reason=" + .commit.verification.reason'
done
```
Expand Down Expand Up @@ -193,8 +202,8 @@ Follow these rules when filling in the template:
- **Related Issue:** Include `Fixes #NNN` or `Closes #NNN` if an issue exists. Remove the section entirely if there is no related issue.
- **Changes:** Bullet list of key changes. Be specific — reference file names, commands, or behaviors that changed.
- **Type of Change:** Check exactly one box. Use `[x]` for checked, `[ ]` for unchecked.
- **Quality Gates:** Check every line that applies to the diff. If tests/docs are not needed or existing coverage is sufficient, include the justification. If sensitive paths changed or a non-success CI check is accepted, record the authorized reviewer, maintainer-approved waiver, approval link, or follow-up issue.
- **Verification:** Check only the boxes for steps you actually ran and confirmed passing, or for Git hooks that passed during normal commit and push. Do not check boxes for steps you skipped or did not verify. The DCO declaration and GitHub verification checkbox is mandatory before PR creation because Step 4 must pass first. For doc-only changes, `npm test` is not required; leave it unchecked unless you ran it.
- **Quality Gates:** Check exactly one tests line and one docs line, then check every other line that applies to the diff. If tests/docs are not needed or existing coverage is sufficient, include the justification. If sensitive paths changed or a non-success CI check is accepted, record the authorized reviewer, maintainer-approved waiver, approval link, or follow-up issue.
- **Verification:** Check only the boxes backed by the requested command/result, justification, normal hook evidence, or fallback evidence. Do not check boxes for steps you skipped or did not verify. The DCO declaration and GitHub verification checkbox is mandatory before PR creation because Step 4 must pass first. For focused changes, leave the broad-gate line unchecked unless you actually ran the applicable command.
- **DCO Sign-Off:** Replace `{name}` and `{email}` with values from `git config user.name` and `git config user.email`.

## Step 7: Create the PR
Expand Down Expand Up @@ -245,8 +254,9 @@ Automated review: no actionable findings / addressed findings / waiting on user
- **Do not invent your own PR body format.** Use `.github/PULL_REQUEST_TEMPLATE.md` exactly.
- **Do not omit sections.** Even if a section is not applicable, keep it with the "Skip if..." comment.
- **Do not check boxes for steps you did not run.** If you did not run `npm run docs`, leave that box unchecked.
- **Do not rerun hook-covered checks by default.** Normal commit and push hooks are valid verification. Use `npx prek run --from-ref main --to-ref HEAD` as the fallback when hooks were skipped, missing, or uncertain.
- **Do not run the full test suite for doc-only changes by default.** Run the docs build instead, and leave `npm test` unchecked unless you actually ran it.
- **Do not rerun hook-covered checks by default.** Normal `pre-commit`, `commit-msg`, and `pre-push` hooks are valid verification. Use `npm run check:diff` once as the fallback when hooks were skipped, missing, or uncertain.
- **Do not run targeted tests more than once per unchanged relevant change set.** Record the passing command and result; rerun when subsequent edits or hook autofixes can affect that behavior.
- **Do not run broad gates for doc-only changes by default.** Run the docs build instead, and leave the broad-gate verification item unchecked unless you actually ran the applicable command.
- **Do not forget the DCO sign-off declaration in the PR body.** CI will reject the PR without it.
- **Do not create PRs with unverified commits.** GitHub must report every PR commit as `Verified` before the PR is opened.
- **Do not rely on maintainers to repair contributor signature history.** If force-push is not allowed and the branch contains an unverified commit, use a fresh branch and fresh PR.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,24 @@ Start with the manifest. Add core code only when the manifest vocabulary cannot

## Verification

Use the narrowest tests that cover the changed behavior:
Build one targeted Vitest invocation from only the files that cover the changed behavior.
Omit unaffected paths from this example, then run the resulting command once per relevant change set:

```bash
npm run build:cli
npm run typecheck:cli
npx vitest run src/lib/messaging/channels/manifests.test.ts src/lib/messaging/channels/metadata.test.ts src/lib/messaging/compiler/manifest-compiler.test.ts
npx vitest run src/lib/messaging/channels/<channel>/hooks
npx vitest run test/messaging-build-applier.test.ts
npx vitest run \
src/lib/messaging/channels/<channel> \
src/lib/messaging/channels/manifests.test.ts \
src/lib/messaging/channels/metadata.test.ts \
src/lib/messaging/compiler/manifest-compiler.test.ts \
test/messaging-build-applier.test.ts
```

Add channel-specific config render, hook, policy, and channel add/remove tests when those surfaces change.
Run `npm run docs` for documentation changes and `npx prek run --files <changed files>` before handoff. If broad hooks expose unrelated failures, report the failure with the targeted passing evidence.
Rerun the targeted command after later edits or hook autofixes that can affect the tested behavior.
Run `npm run docs` for documentation changes.
Commit and push normally so pre-commit handles cheap structural and file-local checks and pre-push runs the path-scoped type checks.
Treat successful hooks as verification and do not rerun their checks manually.
If `pre-commit`, `commit-msg`, or `pre-push` hooks were skipped or unavailable, run `npm run check:diff` once to reproduce those checks.
Refresh `origin/main` first.
Reserve `npm test` for broad runtime or test-harness changes.
Reserve `npm run check` for repo-wide validation or coverage-baseline changes.
10 changes: 5 additions & 5 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not applicable", or waiver entry, add a brief justification on the same line or in the Changes section. -->
<!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. -->
- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
Expand All @@ -27,11 +27,11 @@
- [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you skipped. Doc-only changes do not require npm test unless you ran it. -->
<!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. -->
- [ ] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub
- [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes
- [ ] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [ ] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable
- [ ] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification:
- [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result:
- [ ] Quality Gates section completed with required justifications or waivers
- [ ] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
Expand Down
2 changes: 0 additions & 2 deletions .github/actions/ci-static-checks/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ runs:
shell: bash
run: |
npx prek run --all-files --stage pre-commit \
--skip test-cli \
--skip test-plugin \
--skip source-shape-test-budget \
--skip test-file-size-budget \
--skip test-skills-yaml
Expand Down
21 changes: 11 additions & 10 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
# Usage:
# npx prek install
# npx prek run --all-files
# npx prek run --all-files --stage manual # full CLI/plugin coverage
#
# CI / diff-only runs:
# npx prek run --from-ref <base> --to-ref HEAD
# Diff-only fallback for automatic commit, commit-message, and push checks:
# npm run check:diff
#
# Priority groups (prek runs same-priority hooks in parallel):
# 0 — General file fixers (whitespace, EOF, line endings)
Expand Down Expand Up @@ -240,17 +241,16 @@ repos:
entry: bash -c 'npm run build:cli && npx tsc -p jsconfig.json'
language: system
pass_filenames: false
files: ^(bin|test|scripts)/.*\.js$
files: ^(bin|test|scripts)/.*\.js$|^(jsconfig\.json|package(-lock)?\.json)$
stages: [pre-push]
priority: 10

- id: tsc-cli
name: TypeScript (CLI)
entry: npx tsc -p tsconfig.cli.json
entry: npm run typecheck:cli -- --incremental
language: system
pass_filenames: false
files: ^(bin|scripts|src|test|nemoclaw-blueprint/scripts)/.*\.(ts|tsx)$|^tsconfig\.cli\.json$
always_run: true
files: ^(agents/hermes|bin|scripts|src|test|tools|nemoclaw-blueprint/scripts)/.*\.(ts|tsx|mts|cts|json)$|^\.agents/skills/nemoclaw-maintainer-day/scripts/(check-gates|pra-gate|shared)\.ts$|^nemoclaw/src/(lib/subprocess-env|blueprint/private-networks)\.ts$|^(package(-lock)?\.json|tsconfig\.cli\.json|vitest\.config\.ts)$
stages: [pre-push]
priority: 10

Expand Down Expand Up @@ -279,25 +279,26 @@ repos:
stages: [post-merge, post-checkout]
priority: 10

# ── Priority 20: project-level checks (coverage + ratchet) ─────────────────
# ── Priority 20: project-level checks (full coverage is manual) ────────────
- repo: local
hooks:
- id: test-cli
name: Test (CLI)
entry: >-
bash -c 'node -e "require(\"node:fs\").rmSync(\"dist\", { recursive: true, force: true })" && npm run build:cli && npx tsx scripts/check-dist-sourcemaps.ts dist && npx vitest run --project cli --project integration --coverage --coverage.reporter=text-summary --coverage.reporter=json-summary --coverage.reportsDirectory=coverage/cli --coverage.include="bin/**/*.js" --coverage.include="src/**/*.ts" --coverage.exclude="test/**/*.js" --coverage.exclude="test/**/*.ts" && npx tsx scripts/check-coverage-ratchet.ts coverage/cli/coverage-summary.json ci/coverage-threshold-cli.json "CLI coverage"'
entry: npm run test:coverage:cli
language: system
pass_filenames: false
files: ^(bin/|src/.*\.(ts|tsx|js|mjs|cjs)$|test/.*\.(ts|tsx|js|mjs|cjs)$)
require_serial: true
stages: [manual]
priority: 20

- id: test-plugin
name: Test (plugin)
entry: bash -c 'npx vitest run --project plugin --coverage --coverage.reporter=text-summary --coverage.reporter=json-summary --coverage.reportsDirectory=coverage/plugin --coverage.include="nemoclaw/src/**/*.ts" --coverage.exclude="**/*.test.ts" && npx tsx scripts/check-coverage-ratchet.ts coverage/plugin/coverage-summary.json ci/coverage-threshold-plugin.json "Plugin coverage"'
entry: npm run test:coverage:plugin
language: system
pass_filenames: false
files: ^nemoclaw/
stages: [manual]
priority: 20

- id: source-shape-test-budget
Expand Down
Loading
Loading