fix: derive CLI version from git tags instead of hard-coded package.json#1221
fix: derive CLI version from git tags instead of hard-coded package.json#1221
Conversation
The -v flag and help output previously read the version from package.json, which was hard-coded at 0.1.0 and never updated when new git tags were created. This caused nemoclaw -v to report the wrong version. Changes: - Add bin/lib/version.js: resolves version from git describe, then .version file, then package.json as a last resort - Update bin/nemoclaw.js to use getVersion() instead of pkg.version - Update install.sh resolve_installer_version() with the same git -> .version -> package.json fallback chain - Fetch v* tags into shallow clones during remote install so git describe works reliably, and stamp .version as belt-and-suspenders - Stamp .version in prepublishOnly for npm tarball installs - Add pre-push hook (scripts/check-version-tag-sync.sh) that blocks pushing a v* tag when package.json version doesn't match - Update tests to reflect the new version resolution behavior
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUnifies version resolution to prefer Git tags, then a root Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant GitRepo
participant VersionFile as .version
participant PkgJSON as package.json
User->>CLI: Request version (e.g., --version)
CLI->>GitRepo: git describe --tags --match "v*"
alt Git tags exist
GitRepo-->>CLI: Return tag (e.g., v1.2.3)
CLI->>CLI: Strip leading 'v'
CLI-->>User: Display version
else No git tags
CLI->>VersionFile: Read .version
alt .version exists
VersionFile-->>CLI: Return version
CLI-->>User: Display version
else .version missing
CLI->>PkgJSON: Read version field
PkgJSON-->>CLI: Return version
CLI-->>User: Display version
end
end
sequenceDiagram
participant Dev
participant Git
participant PrePush
participant Validator as check-version-tag-sync.sh
participant Repo
participant PkgJSON
Dev->>Git: git push
Git->>PrePush: Invoke pre-push hooks with pushed refs (stdin)
PrePush->>Validator: Run script with refs
Validator->>Repo: For each refs/tags/v* get tag name and commit SHA
loop For each tag
Validator->>Repo: Read package.json at commit SHA
Repo-->>Validator: package.json content
Validator->>Validator: Compare tag (sans 'v') vs package.json.version
alt Match
Validator-->>PrePush: Log success
else Mismatch
Validator-->>PrePush: Log error
end
end
alt Any mismatch
PrePush-->>Dev: Reject push (non-zero)
else
PrePush-->>Dev: Allow push
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/check-version-tag-sync.sh (1)
24-29: Use^{commit}to peel tag OIDs to commits when readingpackage.json.The pre-push hook receives
local_shadirectly from git, which for annotated tags can be a tag object OID rather than a commit OID. Whilegit show <tag-object>:package.jsonwould fail, this scenario doesn't currently apply since the repository has no v* tags. However, applying the defensive fix ensures the script handles annotated tags correctly if they're used in the future.🔧 Suggested fix
while IFS=' ' read -r local_ref local_sha _remote_ref _remote_sha; do # Only care about v* tag pushes case "$local_ref" in refs/tags/v*) tag="${local_ref#refs/tags/}" - check_tag "$tag" "$local_sha" || errors=$((errors + 1)) + commit_sha="$(git -C "$ROOT" rev-parse "${local_sha}^{commit}" 2>/dev/null || true)" + if [[ -z "$commit_sha" ]]; then + echo "${RED}✗${RESET} Tag ${tag}: could not resolve commit from ${local_sha:0:8}" >&2 + errors=$((errors + 1)) + else + check_tag "$tag" "$commit_sha" || errors=$((errors + 1)) + fi ;; esac doneAlso applies to: 84-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-version-tag-sync.sh` around lines 24 - 29, The git show calls in version_at_commit (and the similar block around lines 84-90) may receive tag object OIDs for annotated tags; update the git invocations to peel tag OIDs to commits by appending ^{commit} to the SHA (e.g., use "${sha}^{commit}" when calling git show) so git reads package.json from the underlying commit even when given an annotated tag OID; change both occurrences (the version_at_commit function and the other git-show usage) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.sh`:
- Around line 29-35: The git command assignment for git_ver can fail under set
-euo pipefail and abort the script; change the assignment in the block that sets
git_ver so the git describe failure doesn't cause exit (e.g., make the command
tolerant by appending "|| true" or using "|| echo ''" after git -C "$SCRIPT_DIR"
describe --tags --match 'v*' 2>/dev/null), keep the sed 's/^v//' processing as
before, and leave the subsequent [[ -n "$git_ver" ]] check and the .version /
package.json fallbacks intact; update the git_ver assignment line only
(referenced symbol: git_ver and the git -C "$SCRIPT_DIR" describe invocation).
---
Nitpick comments:
In `@scripts/check-version-tag-sync.sh`:
- Around line 24-29: The git show calls in version_at_commit (and the similar
block around lines 84-90) may receive tag object OIDs for annotated tags; update
the git invocations to peel tag OIDs to commits by appending ^{commit} to the
SHA (e.g., use "${sha}^{commit}" when calling git show) so git reads
package.json from the underlying commit even when given an annotated tag OID;
change both occurrences (the version_at_commit function and the other git-show
usage) accordingly.
🪄 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: Pro
Run ID: 6857b7f0-9f8d-4a35-8cfd-2fa581579374
📒 Files selected for processing (7)
.gitignore.pre-commit-config.yamlbin/lib/version.jsbin/nemoclaw.jsinstall.shscripts/check-version-tag-sync.shtest/install-preflight.test.js
|
revoke prior approval, please fix build issue + coderabbit feedback, thx |
- snapshot.test.ts: replace forbidden import() type annotation in importOriginal generic with a top-level type-only import - onboard.js: make health-poll count and interval configurable via NEMOCLAW_HEALTH_POLL_COUNT and NEMOCLAW_HEALTH_POLL_INTERVAL env vars (defaults unchanged for production) - cli.test.js: set poll count=1 and interval=0 in test helper to eliminate ~40s of unnecessary sleep in gateway recovery tests - cli.test.js: reduce test timeouts from 25s to 10s accordingly - Apply Prettier formatting fixes from pre-commit hooks
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 2149-2151: The code uses
Number(process.env.NEMOCLAW_HEALTH_POLL_COUNT) || 5 and
Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL) || 2 which drops valid "0" and
accepts invalid numeric shapes; update the parsing to explicitly validate and
sanitize these env vars (e.g., parseInt/Number, check isFinite, clamp to a safe
integer range, and fall back to defaults only when the parsed value is NaN or
out-of-range) before using them as loop bounds/delays (variables:
healthPollCount, healthPollInterval); also change the polling loop (for (...) {
...; if (i < healthPollCount - 1) await sleep(healthPollInterval * 1000); }) so
the code does not sleep after the final attempt; apply the same validation
pattern to the other occurrences mentioned (the additional uses around the same
logic).
🪄 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: Pro
Run ID: 773bd8cb-2368-4920-88ef-07039da96cf0
📒 Files selected for processing (4)
bin/lib/onboard.jsnemoclaw/src/blueprint/snapshot.test.tstest/cli.test.jstest/uninstall.test.js
✅ Files skipped from review due to trivial changes (2)
- nemoclaw/src/blueprint/snapshot.test.ts
- test/uninstall.test.js
| const healthPollCount = Number(process.env.NEMOCLAW_HEALTH_POLL_COUNT) || 5; | ||
| const healthPollInterval = Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL) || 2; | ||
| for (let i = 0; i < healthPollCount; i++) { |
There was a problem hiding this comment.
Validate poll env values before using them as loop bounds/delays.
Using Number(...) || default drops valid "0" values, accepts invalid numeric shapes, and can produce unsafe loop behavior. Also, recovery currently sleeps after the final poll attempt.
Proposed fix
- const healthPollCount = Number(process.env.NEMOCLAW_HEALTH_POLL_COUNT) || 5;
- const healthPollInterval = Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL) || 2;
+ const parsedHealthPollCount = Number.parseInt(
+ process.env.NEMOCLAW_HEALTH_POLL_COUNT ?? "",
+ 10,
+ );
+ const parsedHealthPollInterval = Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL);
+ const healthPollCount =
+ Number.isInteger(parsedHealthPollCount) && parsedHealthPollCount > 0
+ ? parsedHealthPollCount
+ : 5;
+ const healthPollInterval =
+ Number.isFinite(parsedHealthPollInterval) && parsedHealthPollInterval >= 0
+ ? parsedHealthPollInterval
+ : 2;
@@
- const recoveryPollCount = Number(process.env.NEMOCLAW_HEALTH_POLL_COUNT) || 10;
- const recoveryPollInterval = Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL) || 2;
+ const parsedRecoveryPollCount = Number.parseInt(
+ process.env.NEMOCLAW_HEALTH_POLL_COUNT ?? "",
+ 10,
+ );
+ const parsedRecoveryPollInterval = Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL);
+ const recoveryPollCount =
+ Number.isInteger(parsedRecoveryPollCount) && parsedRecoveryPollCount > 0
+ ? parsedRecoveryPollCount
+ : 10;
+ const recoveryPollInterval =
+ Number.isFinite(parsedRecoveryPollInterval) && parsedRecoveryPollInterval >= 0
+ ? parsedRecoveryPollInterval
+ : 2;
@@
- sleep(recoveryPollInterval);
+ if (i < recoveryPollCount - 1) sleep(recoveryPollInterval);Also applies to: 2160-2160, 2242-2244, 2256-2256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bin/lib/onboard.js` around lines 2149 - 2151, The code uses
Number(process.env.NEMOCLAW_HEALTH_POLL_COUNT) || 5 and
Number(process.env.NEMOCLAW_HEALTH_POLL_INTERVAL) || 2 which drops valid "0" and
accepts invalid numeric shapes; update the parsing to explicitly validate and
sanitize these env vars (e.g., parseInt/Number, check isFinite, clamp to a safe
integer range, and fall back to defaults only when the parsed value is NaN or
out-of-range) before using them as loop bounds/delays (variables:
healthPollCount, healthPollInterval); also change the polling loop (for (...) {
...; if (i < healthPollCount - 1) await sleep(healthPollInterval * 1000); }) so
the code does not sleep after the final attempt; apply the same validation
pattern to the other occurrences mentioned (the additional uses around the same
logic).
Apply the same import type fs + importOriginal<typeof fs>() pattern from snapshot.test.ts to runner.test.ts and state.test.ts for consistency.
Replace Number(env) || default with a dedicated envInt() helper that
handles 0 correctly (Number('0') || 5 would silently fall back to 5),
rejects non-finite values, and clamps to non-negative integers.
The recovery loop in recoverGatewayRuntime slept unconditionally after every iteration, including the last one. Guard with the same i < count - 1 check used in startGatewayWithOptions.
Merge origin/main into feat/jetson-orin-nano-support to resolve conflicts from recent changes (NVIDIA#1208, NVIDIA#1200, NVIDIA#836, NVIDIA#1221, NVIDIA#1223). Jetson detection now leverages main's UNIFIED_MEMORY_GPU_TAGS with added jetson flag and /proc/device-tree fallback. All 116 tests pass.
…son (#1221) ## Problem `nemoclaw -v` reported `v0.1.0` — a hard-coded value in `package.json` that was never updated when new `v*` git tags were created. The `install.sh` banner had the same issue. ## Solution Derive the version from git tags at runtime, with a layered fallback chain: | Install path | Primary source | Fallback | |---|---|---| | Dev clone (full repo) | `git describe --tags --match "v*"` | `package.json` | | `install.sh` remote (shallow clone) | `git describe` after explicit `v*` tag fetch | `.version` file stamped during install | | `npm install -g` (published tarball) | `.version` stamped by `prepublishOnly` | `package.json` | ### Pre-push hook Added `scripts/check-version-tag-sync.sh` as a prek pre-push hook. When pushing a `v*` tag, it verifies `package.json` at the tagged commit has a matching version. Regular branch pushes are unaffected. ## Files changed - **`bin/lib/version.js`** (new) — version resolver: git describe → `.version` → `package.json` - **`bin/nemoclaw.js`** — uses `getVersion()` instead of `pkg.version` - **`install.sh`** — `resolve_installer_version()` uses same fallback chain; fetches `v*` tags into shallow clones - **`package.json`** — `prepublishOnly` stamps `.version`; `files` includes it in tarball - **`.gitignore`** — ignores generated `.version` - **`scripts/check-version-tag-sync.sh`** (new) — pre-push guard - **`.pre-commit-config.yaml`** — registers the pre-push hook - **`test/install-preflight.test.js`** — updated for new version resolution ## Testing - All 740 tests pass - Verified all install paths (dev clone, shallow clone, tag divergence) - Verified pre-push hook blocks/passes correctly <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * CLI and installer now determine and display releases using git tags, stamped version files, or package metadata for more reliable version reporting. * Health poll retry counts and intervals for gateway operations are configurable via environment variables. * **Chores** * Added a pre-push check to ensure git tag and package.json version stay in sync. * `.version` files are now ignored. * **Tests** * Tests updated to accept git-describe style versions, use shorter CLI timeouts, and include minor typing/formatting refactors. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…son (NVIDIA#1221) ## Problem `nemoclaw -v` reported `v0.1.0` — a hard-coded value in `package.json` that was never updated when new `v*` git tags were created. The `install.sh` banner had the same issue. ## Solution Derive the version from git tags at runtime, with a layered fallback chain: | Install path | Primary source | Fallback | |---|---|---| | Dev clone (full repo) | `git describe --tags --match "v*"` | `package.json` | | `install.sh` remote (shallow clone) | `git describe` after explicit `v*` tag fetch | `.version` file stamped during install | | `npm install -g` (published tarball) | `.version` stamped by `prepublishOnly` | `package.json` | ### Pre-push hook Added `scripts/check-version-tag-sync.sh` as a prek pre-push hook. When pushing a `v*` tag, it verifies `package.json` at the tagged commit has a matching version. Regular branch pushes are unaffected. ## Files changed - **`bin/lib/version.js`** (new) — version resolver: git describe → `.version` → `package.json` - **`bin/nemoclaw.js`** — uses `getVersion()` instead of `pkg.version` - **`install.sh`** — `resolve_installer_version()` uses same fallback chain; fetches `v*` tags into shallow clones - **`package.json`** — `prepublishOnly` stamps `.version`; `files` includes it in tarball - **`.gitignore`** — ignores generated `.version` - **`scripts/check-version-tag-sync.sh`** (new) — pre-push guard - **`.pre-commit-config.yaml`** — registers the pre-push hook - **`test/install-preflight.test.js`** — updated for new version resolution ## Testing - All 740 tests pass - Verified all install paths (dev clone, shallow clone, tag divergence) - Verified pre-push hook blocks/passes correctly <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * CLI and installer now determine and display releases using git tags, stamped version files, or package metadata for more reliable version reporting. * Health poll retry counts and intervals for gateway operations are configurable via environment variables. * **Chores** * Added a pre-push check to ensure git tag and package.json version stay in sync. * `.version` files are now ignored. * **Tests** * Tests updated to accept git-describe style versions, use shorter CLI timeouts, and include minor typing/formatting refactors. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…son (NVIDIA#1221) ## Problem `nemoclaw -v` reported `v0.1.0` — a hard-coded value in `package.json` that was never updated when new `v*` git tags were created. The `install.sh` banner had the same issue. ## Solution Derive the version from git tags at runtime, with a layered fallback chain: | Install path | Primary source | Fallback | |---|---|---| | Dev clone (full repo) | `git describe --tags --match "v*"` | `package.json` | | `install.sh` remote (shallow clone) | `git describe` after explicit `v*` tag fetch | `.version` file stamped during install | | `npm install -g` (published tarball) | `.version` stamped by `prepublishOnly` | `package.json` | ### Pre-push hook Added `scripts/check-version-tag-sync.sh` as a prek pre-push hook. When pushing a `v*` tag, it verifies `package.json` at the tagged commit has a matching version. Regular branch pushes are unaffected. ## Files changed - **`bin/lib/version.js`** (new) — version resolver: git describe → `.version` → `package.json` - **`bin/nemoclaw.js`** — uses `getVersion()` instead of `pkg.version` - **`install.sh`** — `resolve_installer_version()` uses same fallback chain; fetches `v*` tags into shallow clones - **`package.json`** — `prepublishOnly` stamps `.version`; `files` includes it in tarball - **`.gitignore`** — ignores generated `.version` - **`scripts/check-version-tag-sync.sh`** (new) — pre-push guard - **`.pre-commit-config.yaml`** — registers the pre-push hook - **`test/install-preflight.test.js`** — updated for new version resolution ## Testing - All 740 tests pass - Verified all install paths (dev clone, shallow clone, tag divergence) - Verified pre-push hook blocks/passes correctly <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * CLI and installer now determine and display releases using git tags, stamped version files, or package metadata for more reliable version reporting. * Health poll retry counts and intervals for gateway operations are configurable via environment variables. * **Chores** * Added a pre-push check to ensure git tag and package.json version stay in sync. * `.version` files are now ignored. * **Tests** * Tests updated to accept git-describe style versions, use shorter CLI timeouts, and include minor typing/formatting refactors. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Problem
nemoclaw -vreportedv0.1.0— a hard-coded value inpackage.jsonthat was never updated when newv*git tags were created. Theinstall.shbanner had the same issue.Solution
Derive the version from git tags at runtime, with a layered fallback chain:
git describe --tags --match "v*"package.jsoninstall.shremote (shallow clone)git describeafter explicitv*tag fetch.versionfile stamped during installnpm install -g(published tarball).versionstamped byprepublishOnlypackage.jsonPre-push hook
Added
scripts/check-version-tag-sync.shas a prek pre-push hook. When pushing av*tag, it verifiespackage.jsonat the tagged commit has a matching version. Regular branch pushes are unaffected.Files changed
bin/lib/version.js(new) — version resolver: git describe →.version→package.jsonbin/nemoclaw.js— usesgetVersion()instead ofpkg.versioninstall.sh—resolve_installer_version()uses same fallback chain; fetchesv*tags into shallow clonespackage.json—prepublishOnlystamps.version;filesincludes it in tarball.gitignore— ignores generated.versionscripts/check-version-tag-sync.sh(new) — pre-push guard.pre-commit-config.yaml— registers the pre-push hooktest/install-preflight.test.js— updated for new version resolutionTesting
Summary by CodeRabbit
New Features
Chores
.versionfiles are now ignored.Tests