Skip to content

ci(hooks): streamline local PR verification - #6270

Merged
cv merged 5 commits into
mainfrom
codex/streamline-pr-verification
Jul 4, 2026
Merged

ci(hooks): streamline local PR verification#6270
cv merged 5 commits into
mainfrom
codex/streamline-pr-verification

Conversation

@cv

@cv cv commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Streamline local PR verification by moving full CLI/plugin coverage out of routine pre-commit hooks, while preserving explicit repo-wide coverage and authoritative CI gates. Add one trusted-base fallback for all local hook stages and align contributor guidance, PR automation, and executable contracts with the new workflow.

Changes

  • Move CLI and plugin coverage hooks to the manual stage, expose named coverage scripts, and keep npm run check as the explicit repo-wide pre-commit plus coverage baseline.
  • Make pre-push CLI checking incremental and path-scoped, add missing checked-JavaScript/config triggers, and retain full typecheck/coverage execution in CI.
  • Add npm run check:diff to reproduce pre-commit, commit-msg, and pre-push checks against a refreshed origin/main.
  • Update contributor docs, agent skills, and the PR template to require targeted tests once per relevant change set and to treat successful hooks as evidence.
  • Add behavioral contracts for hook-stage ownership, path selection, coverage commands, trusted-base selection, and CI skip cleanup.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • 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 — 46 focused contract/skill tests; incremental CLI typecheck; source-shape and test-size budgets
  • Full npm test passes (broad runtime changes only) — not run; the repo-wide pre-commit checks and plugin coverage passed, and CLI coverage passed 10,951 tests plus the ratchet under the project-compatible umask 022
  • 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) — passed with 0 errors and 2 pre-existing warnings
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Documentation

    • Updated contributor/onboarding and PR checklist guidance for verification, including diff-based fallback (check:diff), refreshed origin/main guidance, and tighter “broad-gate” vs targeted-test rules.
    • Refined PR template “Quality Gates” and “Verification” checkbox semantics and wording.
  • Tests

    • Strengthened workflow/contract tests to validate staged hook configuration, diff-scoped command sequences, and expected typecheck scoping.
  • Chores

    • Expanded and adjusted QA scripts (including new CLI/plugin coverage checks) and updated static hook skip behavior and pre-commit hook configuration for manual-stage checks.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added the area: docs Documentation, examples, guides, or docs build label Jul 4, 2026
@cv cv self-assigned this Jul 4, 2026
@github-code-quality

github-code-quality Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/streamline-pr-... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/streamline-pr-... 3a1dcae +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the codex/streamline-pr-... branch is 69%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/streamline-pr-... 3a1dcae +/-
src/lib/shields...nsition-lock.ts 87%
src/lib/actions...all/run-plan.ts 81%
src/lib/state/o...oard-session.ts 78%
src/lib/state/sandbox.ts 73%
src/lib/onboard/preflight.ts 71%
src/lib/shields/index.ts 69%
src/lib/onboard...er-gpu-patch.ts 69%
src/lib/policy/index.ts 66%
src/lib/actions...licy-channel.ts 60%
src/lib/onboard.ts 22%

Updated July 04, 2026 08:50 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E is recommended. The diff is limited to contributor documentation/skills, PR template text, CI/static-check configuration, package validation scripts, TypeScript config coverage, and contract tests for those workflows. It does not modify installer/onboarding runtime code, sandbox lifecycle, credentials, security boundaries, network policy assets, inference routing, deployment logic, or real assistant user flows.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: None
Optional E2E targets: None

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • None. No E2E target dispatch is required: the PR changes contributor docs, PR template guidance, general CI/static hook configuration, package check scripts, and non-E2E tests outside test/e2e/. It does not change .github/workflows/e2e.yaml, the E2E target registry/runtime support, live E2E files, fixtures, support helpers, or target-relevant runtime paths.

Optional E2E targets

  • None.

Relevant changed files

  • None.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3b789039-b3a6-4c6a-944f-d09eaac38241

📥 Commits

Reviewing files that changed from the base of the PR and between fe0fdf5 and 3a1dcae.

📒 Files selected for processing (3)
  • .pre-commit-config.yaml
  • test/pr-workflow-contract.test.ts
  • tsconfig.cli.json
🚧 Files skipped from review as they are similar to previous changes (3)
  • tsconfig.cli.json
  • test/pr-workflow-contract.test.ts
  • .pre-commit-config.yaml

📝 Walkthrough

Walkthrough

This PR updates contributor guidance, hooks, scripts, and tests to use origin/main-anchored verification, adds check:diff, and moves CLI/plugin coverage checks into npm scripts. It also revises PR checklist wording and expands contract tests for the new workflow.

Changes

Trusted-base and hook/script consolidation

Layer / File(s) Summary
Coverage and check scripts
package.json
Adds test:coverage:cli/test:coverage:plugin, changes check to run prek across pre-commit and manual stages, and adds check:diff for diff-scoped checks against origin/main.
Pre-commit hook rewiring
.pre-commit-config.yaml, .github/actions/ci-static-checks/action.yaml
Updates usage comments, switches tsc-cli to npm run typecheck:cli with broadened file globs, moves test-cli/test-plugin to manual stage using npm scripts, and removes their skip flags from CI static checks.
Contributor create-PR skill
.agents/skills/nemoclaw-contributor-create-pr/SKILL.md
Refreshes origin/main before branch checks, uses origin/main..HEAD for ahead-count and commit/DCO verification, rewrites hook-evidence/fallback guidance to npm run check:diff, and tightens PR body checkbox rules.
Messaging channel onboarding skill
.agents/skills/nemoclaw-contributor-onboard-messaging-channel/SKILL.md
Replaces multi-command verification example with a single targeted npx vitest run invocation and updated fallback/check guidance.
PR template checklist wording
.github/PULL_REQUEST_TEMPLATE.md
Tightens Quality Gates and Verification checkbox instructions requiring recorded evidence per item.
AGENTS.md and CONTRIBUTING.md guidance
AGENTS.md, CONTRIBUTING.md
Updates quick reference commands, hook descriptions, and PR requirement/verification wording to use origin/main refresh and npm run check:diff fallback.
Doc-only verification guidance
docs/AGENTS.md, docs/CONTRIBUTING.md, fern/AGENTS.md
Revises doc-only PR verification instructions to use npm run check:diff and origin/main refresh instead of npx prek run --all-files.
Test coverage for hook/script contract
test/pr-workflow-contract.test.ts, test/skills-frontmatter.test.ts, tsconfig.cli.json
Expands hook config types, adds a script-invocation logging helper, adds assertions for coverage/diff scripts and file-scoping, tightens skip-flag checks, adds origin/main-based git command assertions, and adds a new file to tsconfig.cli.json includes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#2852: Both PRs modify the test-cli/test-plugin pre-commit hooks in .pre-commit-config.yaml.
  • NVIDIA/NemoClaw#4262: Both PRs update contributor docs and PR checklist guidance in shared workflow files.

Suggested labels: chore

Suggested reviewers: ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: simplifying local PR verification and hook-based workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/streamline-pr-verification

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-3: check:diff script lacks git fetch --prune origin main; then add or justify PRA-T1.
Open items: 7 required · 4 warnings · 3 suggestions · 8 test follow-ups
Since last review: 7 prior items resolved · 1 still applies · 3 new items found

Action checklist

  • PRA-3 Fix: check:diff script lacks git fetch --prune origin main in package.json:39
  • PRA-4 Fix: Contract tests verify command sequences but not runtime behavior with real git refs in test/pr-workflow-contract.test.ts:648
  • PRA-5 Fix: Merge conflict with PR chore(openclaw): upgrade to 2026.6.10 and harden runtime integration #5595 on test/pr-workflow-contract.test.ts in test/pr-workflow-contract.test.ts:1
  • PRA-6 Fix: Merge conflict with PR fix(onboard): diagnose incomplete custom plugin images #6250 on tsconfig.cli.json in tsconfig.cli.json:13
  • PRA-7 Fix: Merge conflict with PR chore(architecture): map and characterize onboarding lifecycle contracts (#6225) #6259 on AGENTS.md in AGENTS.md:1
  • PRA-13 Fix: Acceptance clause 'Add check:diff' incomplete — missing git fetch in package.json:39
  • PRA-14 Fix: Source-of-truth: check:diff fallback lacks integration test proving it works end-to-end in test/pr-workflow-contract.test.ts:648
  • PRA-1 Resolve or justify: Source-of-truth review needed: check:diff fallback (package.json + skills/docs)
  • PRA-2 Resolve or justify: Source-of-truth review needed: git fetch origin/main instruction in docs/skills
  • PRA-8 Resolve or justify: Pre-push typecheck file pattern expansion may increase latency in .pre-commit-config.yaml:180
  • PRA-9 Resolve or justify: Manual and Advanced Setup section doesn't reference new check/check:diff scripts in CONTRIBUTING.md:95
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Contract tests verify command sequences but not runtime behavior with real git refs
  • PRA-T6 Add or justify test follow-up: Source-of-truth: check:diff fallback lacks integration test proving it works end-to-end
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: check:diff fallback (package.json + skills/docs)
  • PRA-10 In-scope improvement: CI static checks action removed --skip test-cli/--skip test-plugin flags (coverage now manual stage) in .github/actions/ci-static-checks/action.yaml:36
  • PRA-11 In-scope improvement: check script runs two separate prek invocations in package.json:38
  • PRA-12 In-scope improvement: Pre-push typecheck expanded to maintainer scripts — verify no untrusted input execution in tsconfig.cli.json:13

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-3 Required correctness package.json:39 Prepend 'git fetch --prune origin main &&' to the check:diff script in package.json, or add a guard that verifies origin/main exists before running hooks.
PRA-4 Required tests test/pr-workflow-contract.test.ts:648 Add an integration test that creates a temporary git repository with commits, runs npm run check:diff (or the underlying prek/commitlint commands), and verifies it passes end-to-end. Alternatively, document why unit-test approach is sufficient and track as known limitation.
PRA-5 Required correctness test/pr-workflow-contract.test.ts:1 Coordinate with PR #5595 author. Rebase this PR on latest main after #5595 merges, or merge main into this branch and resolve conflicts.
PRA-6 Required correctness tsconfig.cli.json:13 Coordinate with PR #6250 author. Rebase on latest main after #6250 merges, or merge main and resolve conflicts.
PRA-7 Required correctness AGENTS.md:1 Coordinate with PR #6259 author. Rebase on latest main after #6259 merges, or merge main and resolve conflicts.
PRA-8 Resolve/justify architecture .pre-commit-config.yaml:180 Monitor pre-push latency in practice. If it becomes problematic, consider splitting into separate targeted typecheck hooks (e.g., separate hook for security-sensitive paths).
PRA-9 Resolve/justify docs CONTRIBUTING.md:95 Add a note in 'Manual and Advanced Setup' referencing 'npm run check' for whole-repo baseline and 'npm run check:diff' for diff-scoped hook verification.
PRA-10 Improvement workflow .github/actions/ci-static-checks/action.yaml:36 Confirm CI coverage jobs (cli-test-shards, plugin-tests) still execute and gate merge. No code change needed.
PRA-11 Improvement correctness package.json:38 Check prek documentation for multi-stage support. If supported, consider 'npx prek run --all-files --stage pre-commit,manual' for simplicity. Otherwise current form is acceptable.
PRA-12 Improvement correctness tsconfig.cli.json:13 Verify the added maintainer scripts (check-gates.ts, pra-gate.ts, shared.ts) only process trusted repository data and don't execute PR-provided content. They appear to be internal tooling so this is likely fine.
PRA-13 Required acceptance package.json:39 Fix PRA-1 by adding git fetch to check:diff script.
PRA-14 Required tests test/pr-workflow-contract.test.ts:648 Add integration test per PRA-2, or document as known limitation with follow-up issue.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-3 Required — check:diff script lacks git fetch --prune origin main

  • Location: package.json:39
  • Category: correctness
  • Problem: The npm run check:diff script runs prek and commitlint against origin/main but does not fetch origin/main first. In a fresh clone without prior git fetch, the command fails with 'unknown revision' error. Documentation instructs contributors to run 'git fetch --prune origin main' first, but the script itself doesn't enforce or include this step.
  • Impact: Contributors in fresh clones or with stale remotes will experience check:diff failures, potentially leading them to skip verification or use incorrect workarounds.
  • Required action: Prepend 'git fetch --prune origin main &&' to the check:diff script in package.json, or add a guard that verifies origin/main exists before running hooks.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: In a fresh clone (git clone <url>; cd NemoClaw; npm install), run 'npm run check:diff' without prior 'git fetch origin main' — observe failure with 'unknown revision'.
  • Missing regression test: Add integration test: create temp git repo with commits on a feature branch, run npm run check:diff, verify it passes without manual fetch.
  • Done when: The required change is committed and verification passes: In a fresh clone (git clone <url>; cd NemoClaw; npm install), run 'npm run check:diff' without prior 'git fetch origin main' — observe failure with 'unknown revision'.
  • Evidence: package.json line 39: check:diff runs 'npx prek run --from-ref origin/main --to-ref HEAD --stage pre-commit && npx commitlint --from origin/main --to HEAD && npx prek run --from-ref origin/main --to-ref HEAD --stage pre-push' without fetch.

PRA-4 Required — Contract tests verify command sequences but not runtime behavior with real git refs

  • Location: test/pr-workflow-contract.test.ts:648
  • Category: tests
  • Problem: New contract test 'executes repo-wide coverage and diff-scoped automatic hook commands' uses runLoggedPackageScript with fake npm/npx/tsx/vitest binaries. It verifies the exact command sequence but does not execute actual git/prek/commitlint against real refs, so it cannot catch issues like missing origin/main fetch or hook execution failures.
  • Impact: False confidence: tests pass even when check:diff would fail in real usage due to missing git fetch or hook misconfiguration.
  • Required action: Add an integration test that creates a temporary git repository with commits, runs npm run check:diff (or the underlying prek/commitlint commands), and verifies it passes end-to-end. Alternatively, document why unit-test approach is sufficient and track as known limitation.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Review test/pr-workflow-contract.test.ts lines 194-210 (runLoggedPackageScript) and lines 640-700 (executes repo-wide coverage test) — fake binaries log commands but don't execute real tools.
  • Missing regression test: Integration test: create temp repo with main and feature branches, make commits, run npm run check:diff, verify exit code 0 and hook stages execute.
  • Done when: The required change is committed and verification passes: Review test/pr-workflow-contract.test.ts lines 194-210 (runLoggedPackageScript) and lines 640-700 (executes repo-wide coverage test) — fake binaries log commands but don't execute real tools.
  • Evidence: test/pr-workflow-contract.test.ts runLoggedPackageScript mocks npm/npx/tsx/vitest; contract test asserts command arrays but not actual execution results.

PRA-5 Required — Merge conflict with PR #5595 on test/pr-workflow-contract.test.ts

PRA-6 Required — Merge conflict with PR #6250 on tsconfig.cli.json

PRA-7 Required — Merge conflict with PR #6259 on AGENTS.md

PRA-13 Required — Acceptance clause 'Add check:diff' incomplete — missing git fetch

  • Location: package.json:39
  • Category: acceptance
  • Problem: The acceptance clause 'Add npm run check:diff to reproduce pre-commit, commit-msg, and pre-push checks against a refreshed origin/main' is partially met. The script exists but doesn't include the required git fetch, making it fail in fresh clones.
  • Impact: The fallback verification workflow is broken for new contributors or fresh checkouts.
  • Required action: Fix PRA-1 by adding git fetch to check:diff script.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run check:diff in fresh clone without prior git fetch — observe failure.
  • Missing regression test: Integration test for fresh clone check:diff execution.
  • Done when: The required change is committed and verification passes: Run check:diff in fresh clone without prior git fetch — observe failure.
  • Evidence: PR body acceptance clause vs package.json:39 script content.

PRA-14 Required — Source-of-truth: check:diff fallback lacks integration test proving it works end-to-end

  • Location: test/pr-workflow-contract.test.ts:648
  • Category: tests
  • Problem: The check:diff fallback is a localized workaround for when hooks are skipped/unavailable. It handles the invalid state of missing hook execution, but lacks an integration test proving it works with real git refs.
  • Impact: Cannot guarantee the fallback actually works in practice; only verifies command sequence.
  • Required action: Add integration test per PRA-2, or document as known limitation with follow-up issue.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check test/pr-workflow-contract.test.ts for integration test using real git repo — none exists.
  • Missing regression test: Integration test: create temp git repo with main/feature branches, run check:diff, verify success.
  • Done when: The required change is committed and verification passes: Check test/pr-workflow-contract.test.ts for integration test using real git repo — none exists.
  • Evidence: runLoggedPackageScript uses fake binaries; no test creates real git repo and runs actual prek/commitlint.
Review findings by urgency: 7 required fixes, 4 items to resolve/justify, 3 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: check:diff fallback (package.json + skills/docs)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Contract test 'executes repo-wide coverage and diff-scoped automatic hook commands' + missing integration test for fresh clone
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Skill/docs instruct 'git fetch origin/main' manually but script doesn't include it; runLoggedPackageScript uses fake binaries

PRA-2 Resolve/justify — Source-of-truth review needed: git fetch origin/main instruction in docs/skills

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Missing: integration test for fresh clone
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Skills/docs instruct 'git fetch origin/main' manually but check:diff script doesn't include it

PRA-8 Resolve/justify — Pre-push typecheck file pattern expansion may increase latency

  • Location: .pre-commit-config.yaml:180
  • Category: architecture
  • Problem: The tsc-cli pre-push hook file pattern expanded significantly to include maintainer scripts (.agents/skills/nemoclaw-maintainer-day/scripts/*.ts), hermes agents, tools/e2e, nemoclaw-blueprint/scripts, and config files. This increases the number of files triggering typecheck on push, potentially increasing latency for contributors.
  • Impact: Contributors may experience slower pre-push hooks, especially on changes touching the newly included paths.
  • Recommended action: Monitor pre-push latency in practice. If it becomes problematic, consider splitting into separate targeted typecheck hooks (e.g., separate hook for security-sensitive paths).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run 'git diff HEAD~1 -- .pre-commit-config.yaml' and examine tsc-cli hook files pattern; measure pre-push time on a typical change touching one of the new paths.
  • Missing regression test: No automated test needed; operational monitoring.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run 'git diff HEAD~1 -- .pre-commit-config.yaml' and examine tsc-cli hook files pattern; measure pre-push time on a typical change touching one of the new paths.
  • Evidence: tsc-cli hook files pattern expanded from '^(bin|scripts|src|test|nemoclaw-blueprint/scripts)/.*\.(ts|tsx)$|^tsconfig\.cli\.json$' to include maintainer scripts, hermes agents, tools/e2e, config files, and more.

PRA-9 Resolve/justify — Manual and Advanced Setup section doesn't reference new check/check:diff scripts

  • Location: CONTRIBUTING.md:95
  • Category: docs
  • Problem: The 'Manual and Advanced Setup' section (lines 95-110) shows troubleshooting commands but doesn't reference the new 'npm run check:diff' or updated 'npm run check' (now runs pre-commit + manual stages) scripts. Contributors troubleshooting hooks will use outdated commands.
  • Impact: Contributors following the manual setup guide will use deprecated commands and miss the new streamlined verification workflow.
  • Recommended action: Add a note in 'Manual and Advanced Setup' referencing 'npm run check' for whole-repo baseline and 'npm run check:diff' for diff-scoped hook verification.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read CONTRIBUTING.md lines 95-110 — the troubleshooting commands list shows old prek install but not new check scripts.
  • Missing regression test: No automated test needed; documentation accuracy.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read CONTRIBUTING.md lines 95-110 — the troubleshooting commands list shows old prek install but not new check scripts.
  • Evidence: CONTRIBUTING.md lines 95-110 show 'npm install', 'npm run build:cli', 'npm run typecheck:cli', './node_modules/.bin/prek install' but not 'npm run check' or 'npm run check:diff'.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-10 Improvement — CI static checks action removed --skip test-cli/--skip test-plugin flags (coverage now manual stage)

  • Location: .github/actions/ci-static-checks/action.yaml:36
  • Category: workflow
  • Problem: CI static checks action removed --skip test-cli and --skip test-plugin flags from prek run (since those hooks moved to manual stage). Contract tests verify the flags are absent. This is correct behavior — coverage now runs as separate manual stage and in CI jobs.
  • Impact: No negative impact; this is the intended behavior change. Coverage runs in dedicated CI jobs (cli-test-shards, plugin-tests) which are required gates.
  • Suggested action: Confirm CI coverage jobs (cli-test-shards, plugin-tests) still execute and gate merge. No code change needed.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check .github/workflows/pr.yaml — cli-test-shards and plugin-tests jobs exist and are required by checks job.
  • Missing regression test: Contract test 'preserves the shared static, build, and coverage gates' verifies skip flags absent.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: .github/actions/ci-static-checks/action.yaml line 36: npx prek run --all-files --stage pre-commit no longer includes --skip test-cli or --skip test-plugin.

PRA-11 Improvement — check script runs two separate prek invocations

  • Location: package.json:38
  • Category: correctness
  • Problem: The 'check' script now runs 'npx prek run --all-files --stage pre-commit && npx prek run --all-files --stage manual' — two separate prek invocations. Could be simplified to a single invocation with multiple stages if prek supports it.
  • Impact: Slightly slower execution and more verbose output; no functional difference.
  • Suggested action: Check prek documentation for multi-stage support. If supported, consider 'npx prek run --all-files --stage pre-commit,manual' for simplicity. Otherwise current form is acceptable.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run 'npx prek --help' or check prek docs for --stage multiple values support.
  • Missing regression test: No regression test needed; simplification only.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: package.json line 38: check script runs two consecutive prek commands.

PRA-12 Improvement — Pre-push typecheck expanded to maintainer scripts — verify no untrusted input execution

  • Location: tsconfig.cli.json:13
  • Category: correctness
  • Problem: tsconfig.cli.json include array adds three maintainer-day script paths at the front (.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts, pra-gate.ts, shared.ts). These TypeScript files now get type-checked on pre-push. This expands the trusted code boundary — ensure these scripts don't execute untrusted input.
  • Impact: If maintainer scripts process PR-provided content, they could introduce security risks. Current evidence suggests they are internal tooling only.
  • Suggested action: Verify the added maintainer scripts (check-gates.ts, pra-gate.ts, shared.ts) only process trusted repository data and don't execute PR-provided content. They appear to be internal tooling so this is likely fine.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read .agents/skills/nemoclaw-maintainer-day/scripts/pra-gate.ts and check-gates.ts — confirm they only read repository files and GitHub API, not untrusted PR content.
  • Missing regression test: No automated test needed; code review of maintainer scripts.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: tsconfig.cli.json include array prepends three maintainer script paths before existing patterns.
Simplification opportunities: 1 possible cut, net -1 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-11 shrink (package.json:38): Two separate 'npx prek run --all-files --stage pre-commit && npx prek run --all-files --stage manual'
    • Replacement: Single 'npx prek run --all-files --stage pre-commit,manual' if prek supports comma-separated stages
    • Net: -1 lines
    • Safety boundary: Must run both pre-commit and manual stages in sequence; order must be preserved
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Integration test: fresh clone -> npm install -> npm run check:diff (without manual git fetch) should pass. Contract tests verify static configuration but not runtime behavior of new workflow (check:diff with real git refs, pre-push typecheck latency with expanded patterns). The PR author ran focused contract/skill tests but not full npm test or fresh-clone check:diff validation.
  • PRA-T2 Runtime validation — Integration test: temp git repo with main and feature branches, make commits, run npm run check:diff, verify exit code 0. Contract tests verify static configuration but not runtime behavior of new workflow (check:diff with real git refs, pre-push typecheck latency with expanded patterns). The PR author ran focused contract/skill tests but not full npm test or fresh-clone check:diff validation.
  • PRA-T3 Runtime validation — Contract test: verify check:diff script includes git fetch --prune origin main. Contract tests verify static configuration but not runtime behavior of new workflow (check:diff with real git refs, pre-push typecheck latency with expanded patterns). The PR author ran focused contract/skill tests but not full npm test or fresh-clone check:diff validation.
  • PRA-T4 Runtime validation — Integration test: verify pre-push typecheck latency with expanded file patterns on typical change. Contract tests verify static configuration but not runtime behavior of new workflow (check:diff with real git refs, pre-push typecheck latency with expanded patterns). The PR author ran focused contract/skill tests but not full npm test or fresh-clone check:diff validation.
  • PRA-T5 Contract tests verify command sequences but not runtime behavior with real git refs — Add an integration test that creates a temporary git repository with commits, runs npm run check:diff (or the underlying prek/commitlint commands), and verifies it passes end-to-end. Alternatively, document why unit-test approach is sufficient and track as known limitation.
  • PRA-T6 Source-of-truth: check:diff fallback lacks integration test proving it works end-to-end — Add integration test per PRA-2, or document as known limitation with follow-up issue.
  • PRA-T7 Acceptance clause — Add npm run check:diff to reproduce pre-commit, commit-msg, and pre-push checks against a refreshed origin/main. — add test evidence or identify existing coverage. package.json:39 adds check:diff script but missing git fetch --prune origin main (PRA-1); skills/docs updated to reference it.
  • PRA-T8 check:diff fallback (package.json + skills/docs) — Contract test 'executes repo-wide coverage and diff-scoped automatic hook commands' + missing integration test for fresh clone. Skill/docs instruct 'git fetch origin/main' manually but script doesn't include it; runLoggedPackageScript uses fake binaries
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: check:diff fallback (package.json + skills/docs)

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Contract test 'executes repo-wide coverage and diff-scoped automatic hook commands' + missing integration test for fresh clone
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Skill/docs instruct 'git fetch origin/main' manually but script doesn't include it; runLoggedPackageScript uses fake binaries

PRA-2 Resolve/justify — Source-of-truth review needed: git fetch origin/main instruction in docs/skills

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as missing.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Missing: integration test for fresh clone
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: Skills/docs instruct 'git fetch origin/main' manually but check:diff script doesn't include it

PRA-3 Required — check:diff script lacks git fetch --prune origin main

  • Location: package.json:39
  • Category: correctness
  • Problem: The npm run check:diff script runs prek and commitlint against origin/main but does not fetch origin/main first. In a fresh clone without prior git fetch, the command fails with 'unknown revision' error. Documentation instructs contributors to run 'git fetch --prune origin main' first, but the script itself doesn't enforce or include this step.
  • Impact: Contributors in fresh clones or with stale remotes will experience check:diff failures, potentially leading them to skip verification or use incorrect workarounds.
  • Required action: Prepend 'git fetch --prune origin main &&' to the check:diff script in package.json, or add a guard that verifies origin/main exists before running hooks.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: In a fresh clone (git clone <url>; cd NemoClaw; npm install), run 'npm run check:diff' without prior 'git fetch origin main' — observe failure with 'unknown revision'.
  • Missing regression test: Add integration test: create temp git repo with commits on a feature branch, run npm run check:diff, verify it passes without manual fetch.
  • Done when: The required change is committed and verification passes: In a fresh clone (git clone <url>; cd NemoClaw; npm install), run 'npm run check:diff' without prior 'git fetch origin main' — observe failure with 'unknown revision'.
  • Evidence: package.json line 39: check:diff runs 'npx prek run --from-ref origin/main --to-ref HEAD --stage pre-commit && npx commitlint --from origin/main --to HEAD && npx prek run --from-ref origin/main --to-ref HEAD --stage pre-push' without fetch.

PRA-4 Required — Contract tests verify command sequences but not runtime behavior with real git refs

  • Location: test/pr-workflow-contract.test.ts:648
  • Category: tests
  • Problem: New contract test 'executes repo-wide coverage and diff-scoped automatic hook commands' uses runLoggedPackageScript with fake npm/npx/tsx/vitest binaries. It verifies the exact command sequence but does not execute actual git/prek/commitlint against real refs, so it cannot catch issues like missing origin/main fetch or hook execution failures.
  • Impact: False confidence: tests pass even when check:diff would fail in real usage due to missing git fetch or hook misconfiguration.
  • Required action: Add an integration test that creates a temporary git repository with commits, runs npm run check:diff (or the underlying prek/commitlint commands), and verifies it passes end-to-end. Alternatively, document why unit-test approach is sufficient and track as known limitation.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Review test/pr-workflow-contract.test.ts lines 194-210 (runLoggedPackageScript) and lines 640-700 (executes repo-wide coverage test) — fake binaries log commands but don't execute real tools.
  • Missing regression test: Integration test: create temp repo with main and feature branches, make commits, run npm run check:diff, verify exit code 0 and hook stages execute.
  • Done when: The required change is committed and verification passes: Review test/pr-workflow-contract.test.ts lines 194-210 (runLoggedPackageScript) and lines 640-700 (executes repo-wide coverage test) — fake binaries log commands but don't execute real tools.
  • Evidence: test/pr-workflow-contract.test.ts runLoggedPackageScript mocks npm/npx/tsx/vitest; contract test asserts command arrays but not actual execution results.

PRA-5 Required — Merge conflict with PR #5595 on test/pr-workflow-contract.test.ts

PRA-6 Required — Merge conflict with PR #6250 on tsconfig.cli.json

PRA-7 Required — Merge conflict with PR #6259 on AGENTS.md

PRA-8 Resolve/justify — Pre-push typecheck file pattern expansion may increase latency

  • Location: .pre-commit-config.yaml:180
  • Category: architecture
  • Problem: The tsc-cli pre-push hook file pattern expanded significantly to include maintainer scripts (.agents/skills/nemoclaw-maintainer-day/scripts/*.ts), hermes agents, tools/e2e, nemoclaw-blueprint/scripts, and config files. This increases the number of files triggering typecheck on push, potentially increasing latency for contributors.
  • Impact: Contributors may experience slower pre-push hooks, especially on changes touching the newly included paths.
  • Recommended action: Monitor pre-push latency in practice. If it becomes problematic, consider splitting into separate targeted typecheck hooks (e.g., separate hook for security-sensitive paths).
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Run 'git diff HEAD~1 -- .pre-commit-config.yaml' and examine tsc-cli hook files pattern; measure pre-push time on a typical change touching one of the new paths.
  • Missing regression test: No automated test needed; operational monitoring.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Run 'git diff HEAD~1 -- .pre-commit-config.yaml' and examine tsc-cli hook files pattern; measure pre-push time on a typical change touching one of the new paths.
  • Evidence: tsc-cli hook files pattern expanded from '^(bin|scripts|src|test|nemoclaw-blueprint/scripts)/.*\.(ts|tsx)$|^tsconfig\.cli\.json$' to include maintainer scripts, hermes agents, tools/e2e, config files, and more.

PRA-9 Resolve/justify — Manual and Advanced Setup section doesn't reference new check/check:diff scripts

  • Location: CONTRIBUTING.md:95
  • Category: docs
  • Problem: The 'Manual and Advanced Setup' section (lines 95-110) shows troubleshooting commands but doesn't reference the new 'npm run check:diff' or updated 'npm run check' (now runs pre-commit + manual stages) scripts. Contributors troubleshooting hooks will use outdated commands.
  • Impact: Contributors following the manual setup guide will use deprecated commands and miss the new streamlined verification workflow.
  • Recommended action: Add a note in 'Manual and Advanced Setup' referencing 'npm run check' for whole-repo baseline and 'npm run check:diff' for diff-scoped hook verification.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read CONTRIBUTING.md lines 95-110 — the troubleshooting commands list shows old prek install but not new check scripts.
  • Missing regression test: No automated test needed; documentation accuracy.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read CONTRIBUTING.md lines 95-110 — the troubleshooting commands list shows old prek install but not new check scripts.
  • Evidence: CONTRIBUTING.md lines 95-110 show 'npm install', 'npm run build:cli', 'npm run typecheck:cli', './node_modules/.bin/prek install' but not 'npm run check' or 'npm run check:diff'.

PRA-10 Improvement — CI static checks action removed --skip test-cli/--skip test-plugin flags (coverage now manual stage)

  • Location: .github/actions/ci-static-checks/action.yaml:36
  • Category: workflow
  • Problem: CI static checks action removed --skip test-cli and --skip test-plugin flags from prek run (since those hooks moved to manual stage). Contract tests verify the flags are absent. This is correct behavior — coverage now runs as separate manual stage and in CI jobs.
  • Impact: No negative impact; this is the intended behavior change. Coverage runs in dedicated CI jobs (cli-test-shards, plugin-tests) which are required gates.
  • Suggested action: Confirm CI coverage jobs (cli-test-shards, plugin-tests) still execute and gate merge. No code change needed.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check .github/workflows/pr.yaml — cli-test-shards and plugin-tests jobs exist and are required by checks job.
  • Missing regression test: Contract test 'preserves the shared static, build, and coverage gates' verifies skip flags absent.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: .github/actions/ci-static-checks/action.yaml line 36: npx prek run --all-files --stage pre-commit no longer includes --skip test-cli or --skip test-plugin.

PRA-11 Improvement — check script runs two separate prek invocations

  • Location: package.json:38
  • Category: correctness
  • Problem: The 'check' script now runs 'npx prek run --all-files --stage pre-commit && npx prek run --all-files --stage manual' — two separate prek invocations. Could be simplified to a single invocation with multiple stages if prek supports it.
  • Impact: Slightly slower execution and more verbose output; no functional difference.
  • Suggested action: Check prek documentation for multi-stage support. If supported, consider 'npx prek run --all-files --stage pre-commit,manual' for simplicity. Otherwise current form is acceptable.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Run 'npx prek --help' or check prek docs for --stage multiple values support.
  • Missing regression test: No regression test needed; simplification only.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: package.json line 38: check script runs two consecutive prek commands.

PRA-12 Improvement — Pre-push typecheck expanded to maintainer scripts — verify no untrusted input execution

  • Location: tsconfig.cli.json:13
  • Category: correctness
  • Problem: tsconfig.cli.json include array adds three maintainer-day script paths at the front (.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts, pra-gate.ts, shared.ts). These TypeScript files now get type-checked on pre-push. This expands the trusted code boundary — ensure these scripts don't execute untrusted input.
  • Impact: If maintainer scripts process PR-provided content, they could introduce security risks. Current evidence suggests they are internal tooling only.
  • Suggested action: Verify the added maintainer scripts (check-gates.ts, pra-gate.ts, shared.ts) only process trusted repository data and don't execute PR-provided content. They appear to be internal tooling so this is likely fine.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Read .agents/skills/nemoclaw-maintainer-day/scripts/pra-gate.ts and check-gates.ts — confirm they only read repository files and GitHub API, not untrusted PR content.
  • Missing regression test: No automated test needed; code review of maintainer scripts.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: tsconfig.cli.json include array prepends three maintainer script paths before existing patterns.

PRA-13 Required — Acceptance clause 'Add check:diff' incomplete — missing git fetch

  • Location: package.json:39
  • Category: acceptance
  • Problem: The acceptance clause 'Add npm run check:diff to reproduce pre-commit, commit-msg, and pre-push checks against a refreshed origin/main' is partially met. The script exists but doesn't include the required git fetch, making it fail in fresh clones.
  • Impact: The fallback verification workflow is broken for new contributors or fresh checkouts.
  • Required action: Fix PRA-1 by adding git fetch to check:diff script.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Run check:diff in fresh clone without prior git fetch — observe failure.
  • Missing regression test: Integration test for fresh clone check:diff execution.
  • Done when: The required change is committed and verification passes: Run check:diff in fresh clone without prior git fetch — observe failure.
  • Evidence: PR body acceptance clause vs package.json:39 script content.

PRA-14 Required — Source-of-truth: check:diff fallback lacks integration test proving it works end-to-end

  • Location: test/pr-workflow-contract.test.ts:648
  • Category: tests
  • Problem: The check:diff fallback is a localized workaround for when hooks are skipped/unavailable. It handles the invalid state of missing hook execution, but lacks an integration test proving it works with real git refs.
  • Impact: Cannot guarantee the fallback actually works in practice; only verifies command sequence.
  • Required action: Add integration test per PRA-2, or document as known limitation with follow-up issue.
  • Expected follow-up: Fix before merge or get explicit maintainer override.
  • Verification: Check test/pr-workflow-contract.test.ts for integration test using real git repo — none exists.
  • Missing regression test: Integration test: create temp git repo with main/feature branches, run check:diff, verify success.
  • Done when: The required change is committed and verification passes: Check test/pr-workflow-contract.test.ts for integration test using real git repo — none exists.
  • Evidence: runLoggedPackageScript uses fake binaries; no test creates real git repo and runs actual prek/commitlint.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: `npm run check:diff` fallback for skipped, missing, failed, or uncertain local hooks.
Open items: 0 required · 2 warnings · 0 suggestions · 7 test follow-ups
Since last review: 0 prior items resolved · 1 still applies · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: `npm run check:diff` fallback for skipped, missing, failed, or uncertain local hooks
  • PRA-2 Resolve or justify: Add a real hook-selection contract for check:diff in package.json:37
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Add a real hook-selection contract for check:diff
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: `npm run check:diff` fallback for skipped, missing, failed, or uncertain local hooks

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify tests package.json:37 Add or identify a dry-run hook-selection contract that exercises the real `prek` selection behavior for representative changed path sets, rather than only asserting the package-script text and regex fragments.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: `npm run check:diff` fallback for skipped, missing, failed, or uncertain local hooks

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Current tests assert package-script command composition and hook regex/stage ownership, but they do not prove real `prek` hook selection for representative changed-file sets.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `.agents/skills/nemoclaw-contributor-create-pr/SKILL.md`, `.pre-commit-config.yaml`, `CONTRIBUTING.md`, and `package.json` all introduce or document `check:diff` as the fallback; the remaining gap is covered by the warning finding for real hook-selection coverage.

PRA-2 Resolve/justify — Add a real hook-selection contract for check:diff

  • Location: package.json:37
  • Category: tests
  • Problem: The PR documents `npm run check:diff` as the single fallback that reproduces `pre-commit`, `commit-msg`, and `pre-push` checks, but the added contract test only logs the static package-script command sequence and separately checks hook regexes. It does not prove that the actual `prek` runner selects the intended hooks for representative diffs.
  • Impact: A contributor could rely on the new fallback while a real changed-path scenario omits an intended local gate, especially around path-scoped pre-push typechecks or docs-only/config/package changes. CI remains authoritative, but the local workflow contract being changed by this PR would be under-tested.
  • Recommended action: Add or identify a dry-run hook-selection contract that exercises the real `prek` selection behavior for representative changed path sets, rather than only asserting the package-script text and regex fragments.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `test/pr-workflow-contract.test.ts` around `executes repo-wide coverage and diff-scoped automatic hook commands`; the shortest check is to confirm whether any test invokes `prek` in dry-run/list mode against synthetic changed paths instead of only using `runLoggedPackageScript`.
  • Missing regression test: Add a behavior test such as `check:diff selects pre-commit, commitlint, and pre-push hooks for TypeScript, JavaScript, package/config, plugin, and docs-only diffs`, covering `tsc-cli`, `tsc-js`, `tsc-plugin`, and `version-tag-sync` selection/non-selection expectations.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `test/pr-workflow-contract.test.ts` around `executes repo-wide coverage and diff-scoped automatic hook commands`; the shortest check is to confirm whether any test invokes `prek` in dry-run/list mode against synthetic changed paths instead of only using `runLoggedPackageScript`.
  • Evidence: `package.json` adds `check:diff` as `npx prek run --from-ref origin/main --to-ref HEAD --stage pre-commit && npx commitlint --from origin/main --to HEAD && npx prek run --from-ref origin/main --to-ref HEAD --stage pre-push`; `test/pr-workflow-contract.test.ts` asserts those argv arrays and regex matches, but does not execute real `prek` selection for changed-file scenarios.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — `check:diff selects pre-commit, commitlint, tsc-cli, and version-tag-sync for a TypeScript source or test diff`.. Changed tests cover static command composition, hook stage ownership, path regexes, trusted CI action boundaries, and contributor guidance. The remaining confidence gap is behavioral: whether the real `prek` runner selects the intended hooks for `check:diff` under representative diffs.
  • PRA-T2 Runtime validation — `check:diff selects tsc-js for checked JavaScript and package/jsconfig diffs without selecting unrelated plugin typechecks`.. Changed tests cover static command composition, hook stage ownership, path regexes, trusted CI action boundaries, and contributor guidance. The remaining confidence gap is behavioral: whether the real `prek` runner selects the intended hooks for `check:diff` under representative diffs.
  • PRA-T3 Runtime validation — `check:diff selects tsc-plugin for plugin diffs and does not select CLI-only typechecks for unrelated plugin paths except the explicitly shared plugin inputs`.. Changed tests cover static command composition, hook stage ownership, path regexes, trusted CI action boundaries, and contributor guidance. The remaining confidence gap is behavioral: whether the real `prek` runner selects the intended hooks for `check:diff` under representative diffs.
  • PRA-T4 Runtime validation — `check:diff handles docs-only diffs without selecting manual coverage or path-scoped typecheck hooks while preserving the intended automatic hook behavior`.. Changed tests cover static command composition, hook stage ownership, path regexes, trusted CI action boundaries, and contributor guidance. The remaining confidence gap is behavioral: whether the real `prek` runner selects the intended hooks for `check:diff` under representative diffs.
  • PRA-T5 Add a real hook-selection contract for check:diff — Add or identify a dry-run hook-selection contract that exercises the real `prek` selection behavior for representative changed path sets, rather than only asserting the package-script text and regex fragments.
  • PRA-T6 Acceptance clause — No linked issue clauses or linked issue comments were available in the deterministic context. — add test evidence or identify existing coverage. `linkedIssues: []` in the validation context, so there are no issue acceptance clauses to map literally to the diff.
  • PRA-T7 `npm run check:diff` fallback for skipped, missing, failed, or uncertain local hooks — Current tests assert package-script command composition and hook regex/stage ownership, but they do not prove real `prek` hook selection for representative changed-file sets.. `.agents/skills/nemoclaw-contributor-create-pr/SKILL.md`, `.pre-commit-config.yaml`, `CONTRIBUTING.md`, and `package.json` all introduce or document `check:diff` as the fallback; the remaining gap is covered by the warning finding for real hook-selection coverage.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: `npm run check:diff` fallback for skipped, missing, failed, or uncertain local hooks

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Current tests assert package-script command composition and hook regex/stage ownership, but they do not prove real `prek` hook selection for representative changed-file sets.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `.agents/skills/nemoclaw-contributor-create-pr/SKILL.md`, `.pre-commit-config.yaml`, `CONTRIBUTING.md`, and `package.json` all introduce or document `check:diff` as the fallback; the remaining gap is covered by the warning finding for real hook-selection coverage.

PRA-2 Resolve/justify — Add a real hook-selection contract for check:diff

  • Location: package.json:37
  • Category: tests
  • Problem: The PR documents `npm run check:diff` as the single fallback that reproduces `pre-commit`, `commit-msg`, and `pre-push` checks, but the added contract test only logs the static package-script command sequence and separately checks hook regexes. It does not prove that the actual `prek` runner selects the intended hooks for representative diffs.
  • Impact: A contributor could rely on the new fallback while a real changed-path scenario omits an intended local gate, especially around path-scoped pre-push typechecks or docs-only/config/package changes. CI remains authoritative, but the local workflow contract being changed by this PR would be under-tested.
  • Recommended action: Add or identify a dry-run hook-selection contract that exercises the real `prek` selection behavior for representative changed path sets, rather than only asserting the package-script text and regex fragments.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `test/pr-workflow-contract.test.ts` around `executes repo-wide coverage and diff-scoped automatic hook commands`; the shortest check is to confirm whether any test invokes `prek` in dry-run/list mode against synthetic changed paths instead of only using `runLoggedPackageScript`.
  • Missing regression test: Add a behavior test such as `check:diff selects pre-commit, commitlint, and pre-push hooks for TypeScript, JavaScript, package/config, plugin, and docs-only diffs`, covering `tsc-cli`, `tsc-js`, `tsc-plugin`, and `version-tag-sync` selection/non-selection expectations.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `test/pr-workflow-contract.test.ts` around `executes repo-wide coverage and diff-scoped automatic hook commands`; the shortest check is to confirm whether any test invokes `prek` in dry-run/list mode against synthetic changed paths instead of only using `runLoggedPackageScript`.
  • Evidence: `package.json` adds `check:diff` as `npx prek run --from-ref origin/main --to-ref HEAD --stage pre-commit && npx commitlint --from origin/main --to HEAD && npx prek run --from-ref origin/main --to-ref HEAD --stage pre-push`; `test/pr-workflow-contract.test.ts` asserts those argv arrays and regex matches, but does not execute real `prek` selection for changed-file scenarios.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

cv added 2 commits July 4, 2026 01:23
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv

cv commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

Automated-review follow-up for head fe0fdf5:

  • PRA-1: No code change. check:diff is the documented fallback after an explicit git fetch origin main; a normal clone already creates origin/main. Keeping refresh separate avoids making a validation command unexpectedly network-mutating or unusable offline.
  • PRA-2 / PRA-T1–T4: I ran the real npm run check:diff on this head; pre-commit, commitlint, and selected pre-push hooks all passed. The contract test executes the package scripts with deterministic fake binaries so it can assert the exact stage/ref sequence. A nested-repository test that runs full hooks and coverage would be slow and environment-sensitive, recreating the duplicate work this PR removes; normal hook execution plus the complete CI matrix provide runtime coverage.
  • PRA-3–PRA-5: GitHub currently reports this PR as MERGEABLE. The cited items are file overlaps with other open PRs, not current merge conflicts. The branch can be refreshed if one of those PRs changes main before this PR merges.
  • PRA-6: The previous CLI typecheck was unconditional. The new matcher reduces executions to project inputs and uses incremental compilation; the two plugin files retained in both matchers are direct cross-project imports.
  • PRA-7: CONTRIBUTING.md already lists both npm run check and npm run check:diff in the command table immediately before the Git hooks section, then explains when to use each in that section.
  • PRA-8: The dedicated plugin coverage job is green, and the five dedicated CLI coverage shards are running. The workflow contract also asserts those independent coverage jobs.

The earlier typecheck-path warning was fixed in 7b514c62 and refined in fe0fdf5; the exact agent-script input and two cross-project imports now match what tsconfig.cli.json actually checks.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tsconfig.cli.json`:
- Line 19: The CLI typecheck scope currently includes only the pra-gate
entrypoint, so check-gates.ts is still excluded from type checking. Update
tsconfig.cli.json so the include pattern covers
.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts alongside
pra-gate.ts, ensuring typecheck:cli and the tsc-cli pre-push hook validate the
actual gate entrypoint. Reference the existing include array in
tsconfig.cli.json and keep the change aligned with the current script-path
pattern.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1e6b30f9-8084-4002-8a2c-700a907e441b

📥 Commits

Reviewing files that changed from the base of the PR and between 6f5ccbc and fe0fdf5.

📒 Files selected for processing (14)
  • .agents/skills/nemoclaw-contributor-create-pr/SKILL.md
  • .agents/skills/nemoclaw-contributor-onboard-messaging-channel/SKILL.md
  • .github/PULL_REQUEST_TEMPLATE.md
  • .github/actions/ci-static-checks/action.yaml
  • .pre-commit-config.yaml
  • AGENTS.md
  • CONTRIBUTING.md
  • docs/AGENTS.md
  • docs/CONTRIBUTING.md
  • fern/AGENTS.md
  • package.json
  • test/pr-workflow-contract.test.ts
  • test/skills-frontmatter.test.ts
  • tsconfig.cli.json
💤 Files with no reviewable changes (1)
  • .github/actions/ci-static-checks/action.yaml

Comment thread tsconfig.cli.json Outdated
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv

cv commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

Current-head follow-up for 3a1dcae9: the disposition in the earlier review follow-up remains applicable. The later commits only make the typecheck matcher accurately cover project and cross-project inputs; they do not change check:diff behavior.

For the advisor source-boundary request: the invalid state is skipped, missing, failed, or uncertain normal hook execution; .pre-commit-config.yaml is the hook source of truth, package.json composes the fallback, and the create-PR skill/guides are consumers. The constraint is to keep successful normal hooks primary and reproduce only the automatic stages against an explicitly refreshed trusted base when fallback is needed. Regression coverage asserts exact commands, stages, refs, and representative path matching; the real npm run check:diff also passed on this branch. The fallback can be removed if the contribution workflow can guarantee verifiable normal-hook execution in every supported environment.

I am not adding a nested-repository hook-selection test: it would execute the same expensive hooks inside the test suite, be environment-sensitive, and recreate the duplicate work this PR is removing. The deterministic command/path contracts, successful real fallback run, normal commit/push hooks, and complete CI matrix provide proportionate coverage. The remaining Nemotron items about fetch placement, open-PR file overlap, command documentation, and coverage-job ownership are addressed by the prior disposition and current GitHub/CI evidence.

@cv
cv merged commit 6bc3e02 into main Jul 4, 2026
48 checks passed
@cv
cv deleted the codex/streamline-pr-verification branch July 4, 2026 08:52
@cv cv added the v0.0.74 label Jul 4, 2026
@ericksoa ericksoa mentioned this pull request Jul 4, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 4, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [#6020](#6020) and
[#5876](#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [#6251](#6251) and
[#5989](#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [#6232](#6232),
[#6082](#6082),
[#6219](#6219),
[#6214](#6214),
[#6215](#6215),
[#6230](#6230), and
[#6260](#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [#6166](#6166),
[#6254](#6254),
[#6265](#6265),
[#6164](#6164), and
[#6017](#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [#6150](#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [#6234](#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [#6129](#6129),
[#5987](#5987),
[#5955](#5955), and
[#6220](#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [#5963](#5963),
[#6050](#6050),
[#6094](#6094),
[#6238](#6238),
[#5988](#5988),
[#6235](#6235),
[#6181](#6181), and
[#5986](#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [#6200](#6200),
[#6248](#6248),
[#6168](#6168),
[#6270](#6270), and
[#5649](#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
<!-- 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:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- 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. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
cv added a commit that referenced this pull request Jul 7, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Run the integration project as a bounded four-worker phase during the
canonical local `npm test`, while keeping CI, coverage, focused
integration, and direct Vitest runs serialized. Isolate two onboarding
fixtures from host-global dashboard ports so the parallel suite remains
deterministic. This is the final cumulative #6245 step after the named
onboarding conversions, representative process-contract work, and
sequenced loader cleanup already merged; the final clean-build Node 22
suite passes in 3:52.03.

## Related Issue
Closes #6245.

## Changes
- Replace the dashboard-exhaustion fixture's real host listeners with a
fake `lsof` while retaining the real CLI, preflight, diagnostic, and
non-zero exit contract.
- Give the restore-intent fixture an explicit existing dashboard forward
so unrelated host port occupancy cannot divert the behavior under test.
- Resolve integration scheduling from npm lifecycle, CI, coverage, and
worker-cap inputs: local `npm test` uses at most four workers in group
1, while every safety-sensitive route stays serial.
- Add a behavior matrix covering local, CI, coverage, focused, direct,
and explicit worker-throttle modes.
- Complete the cumulative #6245 acceptance path after #6276/#6336/#6383
converted the named onboarding hotspots, #6285/#6417 retained
representative process contracts, and #6286/#6299/#6388/#6415 sequenced
loader cleanup after process removal.
- Record the final host-specific timings, hotspot disposition, and
retained process-contract inventory in `test/README.md` as an advisory
acceptance snapshot rather than a permanent CI budget.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: Test fixtures and local
test-runner scheduling changed; NemoClaw commands, configuration,
runtime behavior, and CI/coverage workflows are unchanged.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Independent final-diff
review confirmed that the fake `lsof` preserves the real
CLI/preflight/exit contract, the restore-intent assertions remain
intact, and resolved CI/coverage configurations remain serialized.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- 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. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: Real
CLI exhaustion contract passed; restore-intent passed with all 11
dashboard ports deliberately occupied; scheduling matrix passed 14/14
through the lifecycle-triggered config; `npm run test:projects:check`
reported 1,327 files disjoint across 8 projects.
- [x] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Clean-build Node 22 `npm
test -- --reporter=blob` under the normal `umask 022` passed 1,251 files
and 13,879 tests with 39 skipped, 1 todo, and zero failures in 3:52.03,
down 73% from the issue's 14:19.65 baseline despite a larger suite. The
matching diff-scoped routine pre-commit stage passed in 13.95s. #6270
separately removed full coverage from routine pre-commit while
preserving manual and authoritative CI gates.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Integration test runs now use adaptive scheduling to speed up local
execution while keeping CI/focused runs serialized.

* **Bug Fixes**
* Improved reliability of onboarding regression coverage by simulating
dashboard port exhaustion in a hermetic way.
* Updated onboarding-related fixtures to better match the intended
readiness/exit behavior.

* **Tests**
* Added coverage for integration scheduling behavior (local caps,
invalid inputs, and CI/coverage scenarios).

* **Documentation**
* Added test-suite documentation with a local performance snapshot and
key test hotspots.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Streamline local PR verification by moving full CLI/plugin coverage out
of routine pre-commit hooks, while preserving explicit repo-wide
coverage and authoritative CI gates. Add one trusted-base fallback for
all local hook stages and align contributor guidance, PR automation, and
executable contracts with the new workflow.

## Changes

- Move CLI and plugin coverage hooks to the manual stage, expose named
coverage scripts, and keep `npm run check` as the explicit repo-wide
pre-commit plus coverage baseline.
- Make pre-push CLI checking incremental and path-scoped, add missing
checked-JavaScript/config triggers, and retain full typecheck/coverage
execution in CI.
- Add `npm run check:diff` to reproduce `pre-commit`, `commit-msg`, and
`pre-push` checks against a refreshed `origin/main`.
- Update contributor docs, agent skills, and the PR template to require
targeted tests once per relevant change set and to treat successful
hooks as evidence.
- Add behavioral contracts for hook-stage ownership, path selection,
coverage commands, trusted-base selection, and CI skip cleanup.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] 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. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] 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. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior — 46 focused
contract/skill tests; incremental CLI typecheck; source-shape and
test-size budgets
- [ ] Full `npm test` passes (broad runtime changes only) — not run; the
repo-wide pre-commit checks and plugin coverage passed, and CLI coverage
passed 10,951 tests plus the ratchet under the project-compatible `umask
022`
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — passed
with 0 errors and 2 pre-existing warnings
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Updated contributor/onboarding and PR checklist guidance for
verification, including diff-based fallback (`check:diff`), refreshed
`origin/main` guidance, and tighter “broad-gate” vs targeted-test rules.
* Refined PR template “Quality Gates” and “Verification” checkbox
semantics and wording.

* **Tests**
* Strengthened workflow/contract tests to validate staged hook
configuration, diff-scoped command sequences, and expected typecheck
scoping.

* **Chores**
* Expanded and adjusted QA scripts (including new CLI/plugin coverage
checks) and updated static hook skip behavior and pre-commit hook
configuration for manual-stage checks.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
This PR prepares the user-facing documentation for v0.0.74 before the
release plan is frozen.
It expands the release notes across the 56-commit train and closes
durable documentation gaps found during the pre-tag commit scan.

## Changes
- Expand the `v0.0.74` release notes to cover OpenShell 0.0.72, managed
MCP, progressive tool disclosure, LangChain Deep Agents Code,
onboarding, local inference, messaging, recovery, and contributor
workflows.
- Correct the `destroy` contract for retained per-name volumes,
gateway-unreachable `--force` cleanup, managed MCP ownership, and
same-name recovery.
- Document separate remediation for an unreachable container DNS
resolver versus one that answers with `NXDOMAIN` or `REFUSED`.
- Document the Windows on Arm N1X automatic Ollama safeguard and its
remaining large-model limitations.
- State that messaging conflicts abort rebuild before backup or
deletion, leaving the original sandbox intact.
- Link the agent-runnable value benchmark from the contributor task
index.
- Synchronize generated agent command variants.
- Validate with `npm run docs:sync-agent-variants` and `npm run docs`;
Fern completed with 0 errors and 2 existing warnings.
- Source summary:
- [NVIDIA#6020](NVIDIA#6020) and
[NVIDIA#5876](NVIDIA#5876) ->
`docs/about/release-notes.mdx`: Consolidate the OpenShell 0.0.72 policy
boundary and managed MCP lifecycle.
- [NVIDIA#6251](NVIDIA#6251) and
[NVIDIA#5989](NVIDIA#5989) ->
`docs/about/release-notes.mdx`: Summarize progressive tool disclosure
and sandbox-first inference controls.
- [NVIDIA#6232](NVIDIA#6232),
[NVIDIA#6082](NVIDIA#6082),
[NVIDIA#6219](NVIDIA#6219),
[NVIDIA#6214](NVIDIA#6214),
[NVIDIA#6215](NVIDIA#6215),
[NVIDIA#6230](NVIDIA#6230), and
[NVIDIA#6260](NVIDIA#6260) ->
`docs/about/release-notes.mdx`: Summarize the experimental LangChain
Deep Agents Code status, secret, version, rebuild, snapshot, and MCP
boundaries.
- [NVIDIA#6166](NVIDIA#6166),
[NVIDIA#6254](NVIDIA#6254),
[NVIDIA#6265](NVIDIA#6265),
[NVIDIA#6164](NVIDIA#6164), and
[NVIDIA#6017](NVIDIA#6017) ->
`docs/about/release-notes.mdx`: Summarize BuildKit prebuild, validated
image reuse, bounded readiness, and preflight improvements.
- [NVIDIA#6150](NVIDIA#6150) ->
`docs/about/release-notes.mdx` and `docs/reference/troubleshooting.mdx`:
Separate unreachable-resolver remediation from reachable-but-rejected
DNS responses.
- [NVIDIA#6234](NVIDIA#6234) ->
`docs/about/release-notes.mdx`,
`docs/inference/use-local-inference.mdx`, and
`docs/get-started/windows-preparation.mdx`: Document N1X automatic 9B
selection and the remaining explicit-large-model boundary.
- [NVIDIA#6129](NVIDIA#6129),
[NVIDIA#5987](NVIDIA#5987),
[NVIDIA#5955](NVIDIA#5955), and
[NVIDIA#6220](NVIDIA#6220) ->
`docs/about/release-notes.mdx`,
`docs/manage-sandboxes/messaging-channels.mdx`,
`docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Document messaging policy
persistence, status, and the pre-destructive conflict check.
- [NVIDIA#5963](NVIDIA#5963),
[NVIDIA#6050](NVIDIA#6050),
[NVIDIA#6094](NVIDIA#6094),
[NVIDIA#6238](NVIDIA#6238),
[NVIDIA#5988](NVIDIA#5988),
[NVIDIA#6235](NVIDIA#6235),
[NVIDIA#6181](NVIDIA#6181), and
[NVIDIA#5986](NVIDIA#5986) ->
`docs/about/release-notes.mdx`, `docs/reference/commands.mdx`, and
`docs/reference/commands-nemohermes.mdx`: Summarize day-two recovery and
clarify retained-volume and local-only destroy semantics.
- [NVIDIA#6200](NVIDIA#6200),
[NVIDIA#6248](NVIDIA#6248),
[NVIDIA#6168](NVIDIA#6168),
[NVIDIA#6270](NVIDIA#6270), and
[NVIDIA#5649](NVIDIA#5649) ->
`docs/about/release-notes.mdx` and `CONTRIBUTING.md`: Summarize
contributor setup and verification improvements and expose the advisory
value benchmark.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Quality Gates
<!-- 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:
- [x] Tests not applicable — justification: documentation-only release
preparation; generated-variant synchronization and the Fern docs build
validate the changed pages and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- 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. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: tests
are not applicable to this documentation-only change; `npm run docs`
validates the source and generated routes.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
* Expanded setup guidance for Windows on Arm devices with safer default
local model selection.
* Clarified local inference and sandbox messaging behavior, including
conflict checks before rebuilds and safer recovery steps.
* Updated destroy/rebuild/reference docs with more detailed warnings,
failure handling, and volume-retention guidance.
* Improved troubleshooting instructions for Docker DNS issues with
clearer paths for unreachable vs. blocked resolvers.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Run the integration project as a bounded four-worker phase during the
canonical local `npm test`, while keeping CI, coverage, focused
integration, and direct Vitest runs serialized. Isolate two onboarding
fixtures from host-global dashboard ports so the parallel suite remains
deterministic. This is the final cumulative NVIDIA#6245 step after the named
onboarding conversions, representative process-contract work, and
sequenced loader cleanup already merged; the final clean-build Node 22
suite passes in 3:52.03.

## Related Issue
Closes NVIDIA#6245.

## Changes
- Replace the dashboard-exhaustion fixture's real host listeners with a
fake `lsof` while retaining the real CLI, preflight, diagnostic, and
non-zero exit contract.
- Give the restore-intent fixture an explicit existing dashboard forward
so unrelated host port occupancy cannot divert the behavior under test.
- Resolve integration scheduling from npm lifecycle, CI, coverage, and
worker-cap inputs: local `npm test` uses at most four workers in group
1, while every safety-sensitive route stays serial.
- Add a behavior matrix covering local, CI, coverage, focused, direct,
and explicit worker-throttle modes.
- Complete the cumulative NVIDIA#6245 acceptance path after NVIDIA#6276/NVIDIA#6336/NVIDIA#6383
converted the named onboarding hotspots, NVIDIA#6285/NVIDIA#6417 retained
representative process contracts, and NVIDIA#6286/NVIDIA#6299/NVIDIA#6388/NVIDIA#6415 sequenced
loader cleanup after process removal.
- Record the final host-specific timings, hotspot disposition, and
retained process-contract inventory in `test/README.md` as an advisory
acceptance snapshot rather than a permanent CI budget.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates
<!-- Check exactly one tests line and one docs line. Check other lines
when applicable. Add every requested justification or approval
reference. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: Test fixtures and local
test-runner scheduling changed; NemoClaw commands, configuration,
runtime behavior, and CI/coverage workflows are unchanged.
- [x] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [x] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification: Independent final-diff
review confirmed that the fake `lsof` preserves the real
CLI/preflight/exit contract, the restore-intent assertions remain
intact, and resolved CI/coverage configurations remain serialized.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification
<!-- 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. -->
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: Real
CLI exhaustion contract passed; restore-intent passed with all 11
dashboard ports deliberately occupied; scheduling matrix passed 14/14
through the lifecycle-triggered config; `npm run test:projects:check`
reported 1,327 files disjoint across 8 projects.
- [x] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Clean-build Node 22 `npm
test -- --reporter=blob` under the normal `umask 022` passed 1,251 files
and 13,879 tests with 39 skipped, 1 todo, and zero failures in 3:52.03,
down 73% from the issue's 14:19.65 baseline despite a larger suite. The
matching diff-scoped routine pre-commit stage passed in 13.95s. NVIDIA#6270
separately removed full coverage from routine pre-commit while
preserving manual and authoritative CI gates.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Integration test runs now use adaptive scheduling to speed up local
execution while keeping CI/focused runs serialized.

* **Bug Fixes**
* Improved reliability of onboarding regression coverage by simulating
dashboard port exhaustion in a hermetic way.
* Updated onboarding-related fixtures to better match the intended
readiness/exit behavior.

* **Tests**
* Added coverage for integration scheduling behavior (local caps,
invalid inputs, and CI/coverage scenarios).

* **Documentation**
* Added test-suite documentation with a local performance snapshot and
key test hotspots.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants