fix(install): report installed version from the requested tag - #7480
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 53025a7 in the TypeScript / code-coverage/cliThe overall coverage in commit 53025a7 in the Updated |
📝 WalkthroughWalkthroughThe installer now fetches requested refs into a deterministic local ref, checks out the exact target commit, and stamps ChangesInstaller versioning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant Git
participant VersionFile
Installer->>Git: Fetch requested ref into target ref
Installer->>Git: Checkout target ref
Installer->>VersionFile: Write resolved version
VersionFile-->>Installer: Provide stamped version
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/install-clone-ref.test.ts (1)
132-179: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider covering both installer entrypoints for parity with tests 1 & 2.
This test only runs
resolve_installer_versionagainstCURL_PIPE_INSTALLER(line 159); tests 1 and 2 loop over bothINSTALLER_PAYLOADandCURL_PIPE_INSTALLER. SinceCURL_PIPE_INSTALLERresolves to a different file than the one under review inscripts/install.sh, extending this test to also exerciseINSTALLER_PAYLOADwould confirm the.version-precedence fix is verified on the source-checkout entrypoint as well, not just the curl-pipe one.♻️ Suggested parity fix
- const resolve = () => + const resolve = (installer: string) => spawnSync( "bash", [ "-c", 'source "$INSTALLER_UNDER_TEST"\nprintf START\nresolve_installer_version\nprintf STOP', ], { encoding: "utf8", env: { ...process.env, - INSTALLER_UNDER_TEST: CURL_PIPE_INSTALLER, + INSTALLER_UNDER_TEST: installer, NEMOCLAW_REPO_ROOT: tmp, NEMOCLAW_INSTALL_REF: "", NEMOCLAW_INSTALL_TAG: "", }, }, ); - - fs.writeFileSync(path.join(tmp, ".version"), "0.0.93"); - const withStamp = resolve(); - expect(withStamp.status, withStamp.stderr).toBe(0); - expect(extract(withStamp.stdout)).toBe("0.0.93"); - - fs.rmSync(path.join(tmp, ".version")); - const withoutStamp = resolve(); - expect(withoutStamp.status, withoutStamp.stderr).toBe(0); - expect(extract(withoutStamp.stdout)).toBe("0.0.38"); + + for (const installer of [INSTALLER_PAYLOAD, CURL_PIPE_INSTALLER]) { + fs.writeFileSync(path.join(tmp, ".version"), "0.0.93"); + const withStamp = resolve(installer); + expect(withStamp.status, withStamp.stderr).toBe(0); + expect(extract(withStamp.stdout)).toBe("0.0.93"); + + fs.rmSync(path.join(tmp, ".version")); + const withoutStamp = resolve(installer); + expect(withoutStamp.status, withoutStamp.stderr).toBe(0); + expect(extract(withoutStamp.stdout)).toBe("0.0.38"); + }🤖 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/install-clone-ref.test.ts` around lines 132 - 179, Update the test around the resolve helper in “reports the stamped .version over a mismatched git describe (`#7474`)” to exercise both installer entrypoints, INSTALLER_PAYLOAD and CURL_PIPE_INSTALLER, rather than only CURL_PIPE_INSTALLER. Run the existing stamped and unstamped assertions for each entrypoint while preserving the current expected versions and cleanup behavior.
🤖 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/install.sh`:
- Around line 173-182: Update the top-level install.sh fetch and checkout flow
to match clone_nemoclaw_ref: fetch the requested ref into
refs/nemoclaw-install/target instead of relying on FETCH_HEAD, then check out
that exact target with detached-head advice disabled. Preserve the existing ref
validation and error handling.
---
Nitpick comments:
In `@test/install-clone-ref.test.ts`:
- Around line 132-179: Update the test around the resolve helper in “reports the
stamped .version over a mismatched git describe (`#7474`)” to exercise both
installer entrypoints, INSTALLER_PAYLOAD and CURL_PIPE_INSTALLER, rather than
only CURL_PIPE_INSTALLER. Run the existing stamped and unstamped assertions for
each entrypoint while preserving the current expected versions and cleanup
behavior.
🪄 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: 83ae291f-6995-4f40-b9fc-d7c891dcd1da
📒 Files selected for processing (2)
scripts/install.shtest/install-clone-ref.test.ts
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Fix-forward after infrastructure loss: the exact-head |
|
Fix-forward update: the manual run-control-plane fallback correctly declined because this diff selects non-credentialed |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed exact head 732dbc0 after the signed sync with current main.\n\nEvidence:\n- installer-integration validation passed: 98/98\n- exact-head ordinary CI passed\n- exact-head cloud-onboard E2E passed\n- canonical PR Review Advisor ledger reports 0 blockers, 0 warnings, 0 suggestions and no follow-up needed\n- no unresolved review threads\n- signed sync commit is GitHub Verified and preserves the contributor's original commits\n\nThe standalone Nemotron lane ended with an analysis-execution failure; the canonical primary Terra assessment completed successfully and classifies the PR as informational with no actionable findings.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Advisor PRA-1 is a false positive on exact head 53025a7. The root bootstrap intentionally sources scripts/install.sh when the versioned payload marker is present, so both tested entry points expose resolve_stamped_version and resolve_installer_version. Exact-head verification passes: test/install-clone-ref.test.ts is 4/4, and sourcing install.sh directly exposes resolve_stamped_version and maps refs/tags/v1.2.3 to 1.2.3. No code change is needed; duplicating the payload helper in the thin bootstrap would create a second implementation and future drift. |
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the exact `## v0.0.96` heading. The entry reconciles all 90 first-parent commits since v0.0.95 with all 92 merged PRs in the live `v0.0.96` label ledger and groups the user-visible changes by operator journey. ## Changes - Add the parser-safe dated MDX changelog entry for v0.0.96 with root-absolute links to the focused user guides. - Source summary: - [#7194](#7194) -> `docs/changelog/2026-07-25.mdx`: Document persistent baseline network policy exclusions and their inspection, rebuild, and snapshot behavior. - [#7188](#7188), [#7427](#7427), and [#7546](#7546) -> `docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference routing, keyless loopback endpoints, and provider-marker isolation. - [#7238](#7238) -> `docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider identifier validation before state writes or OpenShell calls, with bounded terminal-safe rejection previews. - [#7319](#7319), [#7274](#7274), [#7528](#7528), [#7353](#7353), and [#7560](#7560) -> `docs/changelog/2026-07-25.mdx`: Document the managed default gateway service, onboarding readiness, and container-runtime identity safeguards. - [#7349](#7349), [#7498](#7498), [#7406](#7406), [#7196](#7196), [#7559](#7559), [#7421](#7421), [#7510](#7510), [#7295](#7295), and [#7565](#7565) -> `docs/changelog/2026-07-25.mdx`: Document gateway-scoped status, lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and fail-closed CLI prompt and command output. - [#7591](#7591) -> `docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP tool-name discovery, its bounded and names-only contract, probe interaction, and rebuild requirement. - [#7305](#7305), [#7480](#7480), [#7471](#7471), [#7365](#7365), and [#7541](#7541) -> `docs/changelog/2026-07-25.mdx`: Document installer version checks, version-tag reporting, license guidance, WSL Ollama selection, and DGX Station vLLM detection. - [#7482](#7482), [#7466](#7466), [#7208](#7208), [#7434](#7434), and [#7586](#7586) -> `docs/changelog/2026-07-25.mdx`: Document Ollama resource details, reasoning precedence, Hermes onboarding behavior, and preserved managed Hermes BuildKit failures. - [#6830](#6830), [#7492](#7492), [#7563](#7563), and [#7582](#7582) -> `docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw production lock, fixed managed-image dependencies, immutable Hermes base adoption, and Hermes image-size reduction. - [#7505](#7505), [#7530](#7530), [#7547](#7547), [#7508](#7508), [#7548](#7548), [#7549](#7549), [#7537](#7537), [#7534](#7534), [#7515](#7515), [#7511](#7511), [#7551](#7551), [#7562](#7562), [#7575](#7575), [#7496](#7496), [#7594](#7594), [#7595](#7595), and [#7599](#7599) -> `docs/changelog/2026-07-25.mdx`: Summarize release validation, transient and bounded dispatch reconciliation, exact pre-tag qualification, identity revalidation, npm-audit retry, sharding, image reuse, timeout, telemetry, and workflow-hardening changes. - Reconciled without separate changelog prose: - [#7539](#7539), [#7526](#7526), [#7507](#7507), [#7506](#7506), [#7519](#7519), [#7516](#7516), [#7396](#7396), [#7254](#7254), [#7583](#7583), [#7596](#7596), and [#7598](#7598): Test-harness or fixture-only changes. - [#7403](#7403), [#7161](#7161), [#6877](#6877), [#7531](#7531), [#7525](#7525), [#7522](#7522), [#7536](#7536), [#7552](#7552), [#7566](#7566), [#7553](#7553), [#7561](#7561), [#7577](#7577), [#7569](#7569), [#7585](#7585), [#7584](#7584), [#7592](#7592), [#7580](#7580), [#7571](#7571), [#7517](#7517), [#7589](#7589), [#7402](#7402), [#7558](#7558), [#7544](#7544), and [#7601](#7601): Dependency, internal recovery, validation, contributor-workflow, E2E optimization, telemetry, or CI trust changes with no separate user-facing release claim. - [#7556](#7556), [#7573](#7573), [#7576](#7576), and [#7578](#7578): Experimental repository-maintainer conflict automation with no canonical user documentation surface. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, version headings, and published links. - [ ] 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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed `docs/changelog/2026-07-25.mdx` at exact head `0f5dedb47` against 90 first-parent release commits and 92 merged PRs labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version heading, literal CLI names, writing style, skip terms, all 20 root-absolute published links, and the accepted #7591 opt-in authenticated discovery bounds. #7544, #7599, and #7601 remain internal or CI-only release-ledger entries. Changelog tests passed 6/6, the docs build passed with 0 errors and two pre-existing Fern warnings, and `npm run check:diff` plus the final diff check passed. - Agent: Codex Desktop documentation-writer subagent <!-- docs-review-head-sha: 0f5dedb --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line 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 — `npx vitest run test/changelog-docs.test.ts`: 6/6 passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this prose-only changelog entry. - [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) — the build passed with 0 errors and 2 existing Fern warnings; the published-route check passed. - [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) — native changelog files use the required parser-safe MDX SPDX comment and no frontmatter. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Persistent network policy exclusions with consistent restore/exclusion reporting across rebuilds/snapshots. * Opt-in MCP tool discovery via `mcp status --tools` with bounded, redacted authenticated traffic. * Improved HTTPS inference switching for custom endpoints and refreshed onboarding/model menu details. * Refined OpenShell gateway defaults for port `8080`, including more reliable readiness checks. * **Bug Fixes** * Prevent incorrect provider/model restoration after compatible-provider update failures. * Preserve managed MCP state after exec loss and tighten gateway/doctor status scoping. * **Tests** * Stronger, fail-closed release validation with hardened evidence/artifact handoff and bounded timeouts/retries. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
The
curl | bashinstaller stamped the installed version fromgit describeon a shallow clone. When a release tag is lightweight and the checkout is not exactly on the tagged commit, describe resolves to a different nearby tag, sonemoclaw --versionreported an older release than the one requested. The installer now pins the checkout to the requested ref and records the version from that ref, so the reported version matches the tag that was installed.Related Issue
Fixes #7474
Changes
clone_nemoclaw_ref: fetch the requested ref into an explicit local ref and check that out, pinning HEAD to the exact object the ref names instead of relying onFETCH_HEAD.resolve_stamped_version: map an immutable version tag to its semver and record it in.version. Mutable refs (lkg,latest) and non-version refs return empty and keep the previousgit describefallback, whose only current consumers are development clones and CI, which have no.versionfile.resolve_installer_version: read the stamped.versionbeforegit describe, since.versionrecords the exact requested tag while describe on a shallow clone can resolve a different nearby tag.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededdocs/get-started/quickstart.mdxalready tells users to select a versioned release withNEMOCLAW_INSTALL_TAG=vX.Y.Zand clear the higher-priorityNEMOCLAW_INSTALL_REF;docs/reference/commands.mdxalready defines--versionas the installed CLI version and documents both selector variables and precedence. The PR changes no command, flag, environment variable, install procedure, or documented output contract, so no docs page needs a change. The current-main topper leaves the effective four-file product diff byte-identical to the independently reviewed diff (SHA-256950459d8c82d4364060853cafe9f27e82820994f8f787b5ff453f47409139872).DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailabletest/install-preflight.test.ts→ 94 passed; combinedtest/install-preflight.test.tsandtest/install-clone-ref.test.ts→ 98 passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
latestandlkg.