feat(contributor): add read-only developer environment doctor - #6109
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
📝 WalkthroughWalkthroughAdds a new read-only contributor environment doctor script (scripts/dev-setup.sh --doctor) that checks toolchain versions, build artifacts, Git configuration, GitHub auth, Docker, and local CLI installation. Exposes it via a new npm run dev:doctor script, updates AGENTS.md and CONTRIBUTING.md, and adds test coverage. ChangesContributor environment doctor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DevSetupScript
participant GitConfig
participant GitHubCLI
participant Docker
User->>DevSetupScript: npm run dev:doctor
DevSetupScript->>GitConfig: check_git_configuration
GitConfig-->>DevSetupScript: identity/signing/hooks result
DevSetupScript->>GitHubCLI: gh auth status
GitHubCLI-->>DevSetupScript: auth result
DevSetupScript->>Docker: docker info
Docker-->>DevSetupScript: CPU/memory/storage
DevSetupScript-->>User: pass/warn/fail summary
Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6109.docs.buildwithfern.com/nemoclaw |
Contributor
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
Contributor
E2E Target RecommendationRequired E2E targets: None Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
apurvvkumaria
marked this pull request as ready for review
July 1, 2026 07:54
cv
approved these changes
Jul 1, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/dev-setup-doctor.test.ts (1)
180-193: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider hashing file contents for the non-mutation check.
readdirSync(..., { recursive: true })only compares directory entries, so a content-only mutation to an existing file (without adding/removing entries) wouldn't be caught. Since "never mutates the checkout" is the doctor's core contract, comparing content hashes (or mtimes) would give stronger assurance.
[optional_refactor]🤖 Prompt for 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. In `@test/dev-setup-doctor.test.ts` around lines 180 - 193, Strengthen the non-mutation assertion in dev-setup-doctor.test by checking file contents, not just directory entries. In the "reports a ready environment without mutating the fixture" test, keep the existing runDoctor and readdirSync coverage but add a content-based snapshot of the fixture before and after (for example via hashing or file mtimes) using the fixture.repo path so changes to existing files are detected. Ensure the assertion still targets the fixture created by createFixture and verifies the checkout remains unchanged after runDoctor.
🤖 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 `@scripts/dev-setup.sh`:
- Line 281: The version verification for uv, Git, GitHub CLI, and hadolint
currently only checks that the commands exist, not that they meet supported
minimum versions. Update the setup checks around check_command in the dev-setup
script to use the same minimum-version validation flow as Node.js, npm, and
Python by comparing the installed versions for uv, git, gh, and hadolint against
the required baselines. Keep the existing tool identifiers (uv, git, gh,
hadolint) so the checks remain easy to locate and maintain.
---
Nitpick comments:
In `@test/dev-setup-doctor.test.ts`:
- Around line 180-193: Strengthen the non-mutation assertion in
dev-setup-doctor.test by checking file contents, not just directory entries. In
the "reports a ready environment without mutating the fixture" test, keep the
existing runDoctor and readdirSync coverage but add a content-based snapshot of
the fixture before and after (for example via hashing or file mtimes) using the
fixture.repo path so changes to existing files are detected. Ensure the
assertion still targets the fixture created by createFixture and verifies the
checkout remains unchanged after runDoctor.
🪄 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: 6efbae36-07f1-4200-9638-d50ef48e6f68
📒 Files selected for processing (5)
AGENTS.mdCONTRIBUTING.mdpackage.jsonscripts/dev-setup.shtest/dev-setup-doctor.test.ts
5 tasks
Collaborator
Author
Advisor follow-up for head
|
wscurran
added
area: cli
16 tasks
This was referenced Jul 2, 2026
apurvvkumaria
added a commit
that referenced
this pull request
Jul 3, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds a single, idempotent contributor setup command and a matching coding-agent skill so a new NemoClaw engineer can prepare a source checkout and understand first-PR requirements from one supported workflow. Keeps setup and repair repository-local, with host-visible CLI exposure and runtime sandbox onboarding available only through explicit opt-in modes. ## Related Issue Fixes #6103 Related to #3827 Builds on #6109 ## Changes - Extend `scripts/dev-setup.sh` with default setup, `--repair`, machine-readable `--doctor --json`, explicit `--expose-cli`, and opt-in `--with-runtime` modes. - Keep setup inside the trusted checkout: reject mutating root overrides, include development dependencies, reuse a local Python 3.11+ interpreter without downloads, and stop on non-local Git hook overrides. - Keep the doctor read-only by invoking only installed TypeScript binaries, and verify the PATH-resolved CLI itself before accepting a link or managed shim. - Add stable `npm run dev:setup` and repository-pinned `npm run agent` aliases. - Add the `nemoclaw-contributor-onboard` skill with intent-specific routing and whole-checkout trust review, then route contributors to it from the skill catalog and `AGENTS.md`. - Update `CONTRIBUTING.md` and `README.md` with the one-command and one-prompt paths, explicit CLI/runtime boundaries, and first-PR signing/DCO requirements. - Expand integration coverage for setup, repair, idempotency, missing dependencies, JSON readiness, authentication redaction, trust boundaries, CLI validation, and runtime delegation. ## 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: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — setup mutation, executable resolution, Python download, Git hook scope, CLI exposure, and trusted-checkout boundaries reviewed with focused regression coverage - [ ] Non-success, skipped, or missing CI check accepted by maintainer — no CI waiver requested; post-push CI is pending ## 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 - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes — all applicable static/format/security hooks and normal pre-push hooks passed; the broad local `test-cli` hook was skipped after its current-main Linux/permission-sensitive tests failed on macOS, so Linux CI remains authoritative - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) — validation completed with 0 errors and 2 existing Fern warnings - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) — no Fern source page changed - [ ] New doc pages include SPDX header and frontmatter (new pages only) — no new doc page added Verification evidence: - `npx vitest run --project integration test/dev-setup-doctor.test.ts` — 38 tests passed. - `npx vitest run --project integration test/skills-frontmatter.test.ts` — 24 tests passed. - `npx vitest run --project cli src/lib/actions/sandbox/rebuild-gateway-drift.test.ts` — 4 tests passed on the merged head with a writable test HOME. - `npm run typecheck:cli`, `npm run test-size:check`, ShellCheck, shfmt, Biome, repository checks, markdownlint, secret scanning, skill validation, source-shape budget, and commitlint passed. - Normal pre-push TypeScript and package-version hooks passed. - `npm run docs` completed with 0 errors and 2 Fern warnings; the documentation-writer review found no Fern source change necessary because this workflow is contributor-facing. - The two new commits are signed and GitHub reports both as `Verified`; DCO CI passes. --- <!-- 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: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a contributor onboarding agent skill and updated first-PR readiness workflow. * Introduced `dev:setup`, `dev:doctor` enhancements (including `--doctor --json`), and an `agent` command. * Added `--with-runtime` mode for cases requiring sandbox/runtime validation. * **Bug Fixes** * Improved setup/repair and readiness reporting with safer prerequisite checks, clearer ordered remediations, and reliable stop-on-failure behavior. * **Documentation** * Updated README, AGENTS.md, and CONTRIBUTING.md to standardize script-driven setup and doctor-guided fixes. * **Tests** * Expanded tests for setup/repair sequencing, JSON output correctness, and runtime-onboarding triggering. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…#6109) <!-- markdownlint-disable MD041 --> ## Summary Adds a read-only contributor environment doctor so NemoClaw developers can identify source-checkout, toolchain, Git, GitHub, and Docker readiness issues before starting work or opening a pull request. This is the first independently useful contributor-onboarding slice from NVIDIA#6103 and complements the existing end-user installer and coding-agent starter prompt. ## Related Issue Closes NVIDIA#6105. ## Changes - Add `./scripts/dev-setup.sh --doctor` and expose it as `npm run dev:doctor`. - Check supported tools, repository dependencies, fresh build artifacts, prek hooks, contributor identity and signing, GitHub authentication, Docker availability/resources, and local CLI linkage without changing the host. - Align Docker results with the published 4-vCPU/8-GiB minimum and 16-GiB memory recommendation. - Add controlled integration fixtures for ready, missing, stale, invalid, authentication, redaction, runtime, and exit-code behavior. - Document the contributor readiness workflow and correct `uv sync` to run from the repository root. ## 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 - [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: - [x] 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 - [x] 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 - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [ ] 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) Verification evidence: - `npx vitest run --project integration test/dev-setup-doctor.test.ts` — 11 tests passed. - `npm run typecheck:cli`, `npm run checks`, `npm run test:titles:check`, and `npm run test-size:check` passed. - shfmt, ShellCheck, Biome, markdownlint, repository checks, commitlint, secret scanning, and the normal pre-push hooks passed. - `npm run docs` completed with 0 errors and 2 existing Fern warnings. - The final commit skipped the full `test-cli` pre-commit hook after its supported-toolchain run exposed five unrelated macOS integration failures in Deep Agents TUI, gateway-supervisor Python startup, and sandbox auto-pair approval tests. The new doctor integration suite passes independently; CI confirmation and any required maintainer acceptance remain pending while this PR is a draft. --- <!-- 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: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new `dev:doctor` check to validate contributor environment readiness. * Expanded setup guidance with a clear read-only environment verification workflow. * **Bug Fixes** * Updated install instructions to remove an extra directory change. * Clarified setup steps for handling environment check failures and reruns. * **Documentation** * Revised contributor and setup docs to point to the new environment check and updated pre-change workflow. <!-- 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 Adds a single, idempotent contributor setup command and a matching coding-agent skill so a new NemoClaw engineer can prepare a source checkout and understand first-PR requirements from one supported workflow. Keeps setup and repair repository-local, with host-visible CLI exposure and runtime sandbox onboarding available only through explicit opt-in modes. ## Related Issue Fixes NVIDIA#6103 Related to NVIDIA#3827 Builds on NVIDIA#6109 ## Changes - Extend `scripts/dev-setup.sh` with default setup, `--repair`, machine-readable `--doctor --json`, explicit `--expose-cli`, and opt-in `--with-runtime` modes. - Keep setup inside the trusted checkout: reject mutating root overrides, include development dependencies, reuse a local Python 3.11+ interpreter without downloads, and stop on non-local Git hook overrides. - Keep the doctor read-only by invoking only installed TypeScript binaries, and verify the PATH-resolved CLI itself before accepting a link or managed shim. - Add stable `npm run dev:setup` and repository-pinned `npm run agent` aliases. - Add the `nemoclaw-contributor-onboard` skill with intent-specific routing and whole-checkout trust review, then route contributors to it from the skill catalog and `AGENTS.md`. - Update `CONTRIBUTING.md` and `README.md` with the one-command and one-prompt paths, explicit CLI/runtime boundaries, and first-PR signing/DCO requirements. - Expand integration coverage for setup, repair, idempotency, missing dependencies, JSON readiness, authentication redaction, trust boundaries, CLI validation, and runtime delegation. ## 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: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — setup mutation, executable resolution, Python download, Git hook scope, CLI exposure, and trusted-checkout boundaries reviewed with focused regression coverage - [ ] Non-success, skipped, or missing CI check accepted by maintainer — no CI waiver requested; post-push CI is pending ## 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 - [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes — all applicable static/format/security hooks and normal pre-push hooks passed; the broad local `test-cli` hook was skipped after its current-main Linux/permission-sensitive tests failed on macOS, so Linux CI remains authoritative - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) — validation completed with 0 errors and 2 existing Fern warnings - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) — no Fern source page changed - [ ] New doc pages include SPDX header and frontmatter (new pages only) — no new doc page added Verification evidence: - `npx vitest run --project integration test/dev-setup-doctor.test.ts` — 38 tests passed. - `npx vitest run --project integration test/skills-frontmatter.test.ts` — 24 tests passed. - `npx vitest run --project cli src/lib/actions/sandbox/rebuild-gateway-drift.test.ts` — 4 tests passed on the merged head with a writable test HOME. - `npm run typecheck:cli`, `npm run test-size:check`, ShellCheck, shfmt, Biome, repository checks, markdownlint, secret scanning, skill validation, source-shape budget, and commitlint passed. - Normal pre-push TypeScript and package-version hooks passed. - `npm run docs` completed with 0 errors and 2 Fern warnings; the documentation-writer review found no Fern source change necessary because this workflow is contributor-facing. - The two new commits are signed and GitHub reports both as `Verified`; DCO CI passes. --- <!-- 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: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a contributor onboarding agent skill and updated first-PR readiness workflow. * Introduced `dev:setup`, `dev:doctor` enhancements (including `--doctor --json`), and an `agent` command. * Added `--with-runtime` mode for cases requiring sandbox/runtime validation. * **Bug Fixes** * Improved setup/repair and readiness reporting with safer prerequisite checks, clearer ordered remediations, and reliable stop-on-failure behavior. * **Documentation** * Updated README, AGENTS.md, and CONTRIBUTING.md to standardize script-driven setup and doctor-guided fixes. * **Tests** * Expanded tests for setup/repair sequencing, JSON output correctness, and runtime-onboarding triggering. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a read-only contributor environment doctor so NemoClaw developers can identify source-checkout, toolchain, Git, GitHub, and Docker readiness issues before starting work or opening a pull request. This is the first independently useful contributor-onboarding slice from #6103 and complements the existing end-user installer and coding-agent starter prompt.
Related Issue
Closes #6105.
Changes
./scripts/dev-setup.sh --doctorand expose it asnpm run dev:doctor.uv syncto run from the repository root.Type of Change
Quality Gates
Verification
Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Verification evidence:
npx vitest run --project integration test/dev-setup-doctor.test.ts— 11 tests passed.npm run typecheck:cli,npm run checks,npm run test:titles:check, andnpm run test-size:checkpassed.npm run docscompleted with 0 errors and 2 existing Fern warnings.test-clipre-commit hook after its supported-toolchain run exposed five unrelated macOS integration failures in Deep Agents TUI, gateway-supervisor Python startup, and sandbox auto-pair approval tests. The new doctor integration suite passes independently; CI confirmation and any required maintainer acceptance remain pending while this PR is a draft.Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
New Features
dev:doctorcheck to validate contributor environment readiness.Bug Fixes
Documentation