fix(cli): version and updater checks#155
Open
coryrylan wants to merge 1 commit into
Open
Conversation
|
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:
📝 WalkthroughWalkthroughThe PR centralizes Elements environment injection, updates CLI build metadata generation, refactors CLI exit handling, tightens update checks, and changes global skill installation to overwrite existing files without prompting. ChangesRelease and CLI flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/cli/src/utils.ts (1)
145-150: 🎯 Functional Correctness | 🟠 MajorTighten
isReportto validate report entries.
isReport()accepts any truthystatus/message, so malformed data can be narrowed toReportandexitCodeForResult()can return success. Require each entry to be an object,messageto be a string, andstatusto match the supported report statuses.🤖 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 `@projects/cli/src/utils.ts` around lines 145 - 150, The isReport guard is too permissive and can incorrectly narrow malformed data to Report. Update isReport to validate each entry in the object is itself an object, ensure message is a string, and verify status matches the supported report statuses instead of just being truthy. Keep the check localized in isReport so exitCodeForResult continues to rely on a correctly validated Report shape.Source: Coding guidelines
🤖 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 `@projects/cli/src/install.test.ts`:
- Around line 760-782: In the install test that calls spawnSync for
writeInstallManifest, assert the subprocess result succeeded before reading and
parsing dist/manifest.json. Move the check on result.status (and include
result.stderr in the failure path if needed) ahead of the readFile/JSON.parse
step in the test using createInstallerContext and writeInstallManifest, so
failures surface the actual exit error instead of a file-not-found.
In `@projects/cli/src/utils.ts`:
- Line 185: The formatted result in the rendering logic is appending the debug
metadata directly after the prior content, so update the formatting in the utils
rendering path to prepend a separator before the debug block. Locate the
concatenation that adds the [debug] metadata in the result-building flow and
ensure it starts on a new line so the output is separated cleanly from the
rendered content.
---
Outside diff comments:
In `@projects/cli/src/utils.ts`:
- Around line 145-150: The isReport guard is too permissive and can incorrectly
narrow malformed data to Report. Update isReport to validate each entry in the
object is itself an object, ensure message is a string, and verify status
matches the supported report statuses instead of just being truthy. Keep the
check localized in isReport so exitCodeForResult continues to rely on a
correctly validated Report shape.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 872843f0-0461-4b73-9063-626b276e6d00
📒 Files selected for processing (20)
projects/cli/scripts/write-install-manifest.mjsprojects/cli/src/index.tsprojects/cli/src/install.test.tsprojects/cli/src/install.tsprojects/cli/src/update.test.tsprojects/cli/src/update.tsprojects/cli/src/utils.test.tsprojects/cli/src/utils.tsprojects/cli/vite.config.tsprojects/internals/tools/src/api/utils.tsprojects/internals/tools/src/cli/utils.test.tsprojects/internals/tools/src/cli/utils.tsprojects/internals/tools/src/playground/utils.test.tsprojects/internals/tools/src/playground/utils.tsprojects/internals/vite/src/configs/build.jsprojects/internals/vite/src/configs/build.node.jsprojects/internals/vite/src/configs/env.jsprojects/internals/vite/src/configs/test.jsprojects/internals/vite/src/configs/test.node.jsprojects/site/src/_11ty/utils/env.js
coryrylan
force-pushed
the
topic-cli-version-updater
branch
from
9cd318b to
b9fc8cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@projects/cli/src/update.test.ts`:
- Around line 58-69: Add a regression test in fetchLatestSha coverage for a
manifest that returns an empty sha string, since the current test only covers a
missing sha shape. Update the existing test suite in update.test.ts alongside
the fetchLatestSha cases to stub fetch with a manifest containing sha: '' and
assert it rejects with the same failure path as the missing-sha case, so the
runtime guard in fetchLatestSha stays enforced for both invalid manifest shapes.
In `@projects/cli/src/update.ts`:
- Around line 24-28: The SHA guard in the manifest-fetching logic is too loose
because the `sha` value can be an empty string and still pass. Update the `sha`
validation in the update flow so the `sha` returned from `res.json()` must be a
non-empty string before returning it, and continue throwing the missing-SHA
error for blank values; this change should be applied in the manifest parsing
path used by `isUpdateAvailable()` so invalid manifests do not get treated as
“no update available.”
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: dc3c67f8-cb01-45f2-b49c-5749b00c7bd5
📒 Files selected for processing (20)
projects/cli/scripts/write-install-manifest.mjsprojects/cli/src/index.tsprojects/cli/src/install.test.tsprojects/cli/src/install.tsprojects/cli/src/update.test.tsprojects/cli/src/update.tsprojects/cli/src/utils.test.tsprojects/cli/src/utils.tsprojects/cli/vite.config.tsprojects/internals/tools/src/api/utils.tsprojects/internals/tools/src/cli/utils.test.tsprojects/internals/tools/src/cli/utils.tsprojects/internals/tools/src/playground/utils.test.tsprojects/internals/tools/src/playground/utils.tsprojects/internals/vite/src/configs/build.jsprojects/internals/vite/src/configs/build.node.jsprojects/internals/vite/src/configs/env.jsprojects/internals/vite/src/configs/test.jsprojects/internals/vite/src/configs/test.node.jsprojects/site/src/_11ty/utils/env.js
Comment on lines
+58
to
+69
| it('should throw error when manifest is missing sha', async () => { | ||
| vi.stubGlobal( | ||
| 'fetch', | ||
| vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: () => Promise.resolve({ version: '2.1.0' }) | ||
| }) | ||
| ); | ||
|
|
||
| await expect(fetchLatestSha()).rejects.toThrow('Failed to fetch latest build info'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a blank-sha regression case.
This covers sha being absent, but '' is another invalid manifest shape that should be pinned down once the runtime guard is tightened. Otherwise the remaining bad-manifest path can regress unnoticed.
🤖 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 `@projects/cli/src/update.test.ts` around lines 58 - 69, Add a regression test
in fetchLatestSha coverage for a manifest that returns an empty sha string,
since the current test only covers a missing sha shape. Update the existing test
suite in update.test.ts alongside the fetchLatestSha cases to stub fetch with a
manifest containing sha: '' and assert it rejects with the same failure path as
the missing-sha case, so the runtime guard in fetchLatestSha stays enforced for
both invalid manifest shapes.
Comment on lines
+24
to
+28
| const { sha } = (await res.json()) as { sha?: string }; | ||
| if (typeof sha !== 'string') { | ||
| throw new Error('Missing sha'); | ||
| } | ||
| return sha; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject blank manifest SHAs too.
typeof sha === 'string' still accepts '', which means an invalid manifest can pass this guard and then get treated downstream as "no update available" because isUpdateAvailable() only reports when latestSha !== ''. That leaves the missing-SHA failure mode only partially covered.
Proposed fix
- if (typeof sha !== 'string') {
+ if (typeof sha !== 'string' || sha.trim() === '') {
throw new Error('Missing sha');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { sha } = (await res.json()) as { sha?: string }; | |
| if (typeof sha !== 'string') { | |
| throw new Error('Missing sha'); | |
| } | |
| return sha; | |
| const { sha } = (await res.json()) as { sha?: string }; | |
| if (typeof sha !== 'string' || sha.trim() === '') { | |
| throw new Error('Missing sha'); | |
| } | |
| return sha; |
🤖 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 `@projects/cli/src/update.ts` around lines 24 - 28, The SHA guard in the
manifest-fetching logic is too loose because the `sha` value can be an empty
string and still pass. Update the `sha` validation in the update flow so the
`sha` returned from `res.json()` must be a non-empty string before returning it,
and continue throwing the missing-SHA error for blank values; this change should
be applied in the manifest parsing path used by `isUpdateAvailable()` so invalid
manifests do not get treated as “no update available.”
b9fc8cd to
76d2980
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
projects/cli/src/utils.ts (1)
185-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMissing separator before
[debug]block.Line 185 still appends
[debug]directly onto the rendered result (...}[debug]), with no leading newline, despite the prior fix being marked as addressed. Prefix the debug block with\n.Suggested fix
- formattedResult += `[debug]\n[command]: ${tool.metadata.command}`; + formattedResult += `\n[debug]\n[command]: ${tool.metadata.command}`;🤖 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 `@projects/cli/src/utils.ts` at line 185, The rendered output in utils.ts still appends the [debug] block directly after the main result, so update the formatting in the code that builds formattedResult to prefix the debug section with a leading newline before [debug]. Use the existing string assembly around formattedResult and tool.metadata.command to keep the debug block separated from the prior content.
🤖 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 `@projects/cli/scripts/write-install-manifest.mjs`:
- Around line 29-34: The getBundledConstant helper builds a RegExp from the
unescaped name argument, so update that pattern construction to safely escape
any regex metacharacters before interpolation. Keep the lookup logic in
getBundledConstant and preserve the missing-constant error path, but make the
guard/message reflect that this is a no-match case rather than a falsy value
check.
In `@projects/cli/src/install.ts`:
- Around line 398-405: The install flow in installSkill should keep the
CI/non-interactive policy check from shouldInstallGlobalElementsSkill(context)
separate from the existing-file shortcut. Call the policy gate first so CI still
blocks global installation, then use hasExistingSkill only to decide whether to
skip the prompt and whether to avoid overwriting an existing SKILL.md without
confirmation. Preserve the behavior around writeGlobalElementsSkill(skillPath)
and the Installed/Updated log message, but make sure reruns cannot bypass the
gate or silently replace a preexisting file.
---
Duplicate comments:
In `@projects/cli/src/utils.ts`:
- Line 185: The rendered output in utils.ts still appends the [debug] block
directly after the main result, so update the formatting in the code that builds
formattedResult to prefix the debug section with a leading newline before
[debug]. Use the existing string assembly around formattedResult and
tool.metadata.command to keep the debug block separated from the prior content.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 983fe57d-b011-4197-a3cc-586d20e1abac
📒 Files selected for processing (21)
projects/cli/scripts/write-install-manifest.mjsprojects/cli/src/index.tsprojects/cli/src/install.test.tsprojects/cli/src/install.tsprojects/cli/src/update.test.tsprojects/cli/src/update.tsprojects/cli/src/utils.test.tsprojects/cli/src/utils.tsprojects/cli/vite.config.tsprojects/internals/tools/src/api/utils.tsprojects/internals/tools/src/cli/utils.test.tsprojects/internals/tools/src/cli/utils.tsprojects/internals/tools/src/playground/utils.test.tsprojects/internals/tools/src/playground/utils.tsprojects/internals/vite/src/configs/build.jsprojects/internals/vite/src/configs/build.node.jsprojects/internals/vite/src/configs/env.jsprojects/internals/vite/src/configs/test.jsprojects/internals/vite/src/configs/test.node.jsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.ts
💤 Files with no reviewable changes (1)
- projects/site/src/_11ty/utils/env.test.ts
Comment on lines
+29
to
+34
| function getBundledConstant(content, name) { | ||
| const value = content.match(new RegExp(`(?:var|const|let)\\s+${name}\\s*=\\s*["']([^"']+)["']`))?.[1]; | ||
| if (!value) { | ||
| throw new Error(`Missing bundled ${name} constant`); | ||
| } | ||
| return value; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Minor: escape name before building the RegExp.
name is interpolated unescaped into the pattern. Today the only callers pass BUILD_SHA/VERSION (regex-safe), but a future caller with regex metacharacters would silently produce a wrong pattern. Also consider that !value rejects a legitimately empty constant — though the [^"']+ group already requires at least one character, so the guard is effectively a missing-match check; a clearer message would help.
🤖 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 `@projects/cli/scripts/write-install-manifest.mjs` around lines 29 - 34, The
getBundledConstant helper builds a RegExp from the unescaped name argument, so
update that pattern construction to safely escape any regex metacharacters
before interpolation. Keep the lookup logic in getBundledConstant and preserve
the missing-constant error path, but make the guard/message reflect that this is
a no-match case rather than a falsy value check.
Comment on lines
+398
to
+405
| const hasExistingSkill = existsSync(skillPath); | ||
| if (!hasExistingSkill && !(await shouldInstallGlobalElementsSkill(context))) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| await writeGlobalElementsSkill(skillPath); | ||
| context.log(`Installed agent skill at ${skillPath}`); | ||
| context.log(`${hasExistingSkill ? 'Updated' : 'Installed'} agent skill at ${skillPath}`); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the CI/install-policy gate separate from the prompt shortcut.
Lines 398-400 now skip shouldInstallGlobalElementsSkill() whenever SKILL.md already exists, but that helper is also the only place that blocks global installs in CI. As written, a rerun in CI will overwrite an existing ~/.agents/skills/elements/SKILL.md without any prompt, and it will silently replace any preexisting file at that path. Preserve the non-interactive policy check before the hasExistingSkill fast path, then use the existence check only to suppress the prompt.
Suggested fix
async function installGlobalElementsSkill(context: InstallContext): Promise<void> {
const skillPath = getGlobalElementsSkillPath(context.env, context.platform);
if (!skillPath) {
context.warn('Could not install Elements agent skill. HOME is not set.');
return;
}
+ if (context.env.CI) {
+ return;
+ }
+
const hasExistingSkill = existsSync(skillPath);
if (!hasExistingSkill && !(await shouldInstallGlobalElementsSkill(context))) {
return;
}
try {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const hasExistingSkill = existsSync(skillPath); | |
| if (!hasExistingSkill && !(await shouldInstallGlobalElementsSkill(context))) { | |
| return; | |
| } | |
| try { | |
| await writeGlobalElementsSkill(skillPath); | |
| context.log(`Installed agent skill at ${skillPath}`); | |
| context.log(`${hasExistingSkill ? 'Updated' : 'Installed'} agent skill at ${skillPath}`); | |
| const skillPath = getGlobalElementsSkillPath(context.env, context.platform); | |
| if (!skillPath) { | |
| context.warn('Could not install Elements agent skill. HOME is not set.'); | |
| return; | |
| } | |
| if (context.env.CI) { | |
| return; | |
| } | |
| const hasExistingSkill = existsSync(skillPath); | |
| if (!hasExistingSkill && !(await shouldInstallGlobalElementsSkill(context))) { | |
| return; | |
| } | |
| try { | |
| await writeGlobalElementsSkill(skillPath); | |
| context.log(`${hasExistingSkill ? 'Updated' : 'Installed'} agent skill at ${skillPath}`); |
🤖 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 `@projects/cli/src/install.ts` around lines 398 - 405, The install flow in
installSkill should keep the CI/non-interactive policy check from
shouldInstallGlobalElementsSkill(context) separate from the existing-file
shortcut. Call the policy gate first so CI still blocks global installation,
then use hasExistingSkill only to decide whether to skip the prompt and whether
to avoid overwriting an existing SKILL.md without confirmation. Preserve the
behavior around writeGlobalElementsSkill(skillPath) and the Installed/Updated
log message, but make sure reruns cannot bypass the gate or silently replace a
preexisting file.
76d2980 to
42deb6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@projects/cli/src/utils.ts`:
- Around line 47-52: `runAsyncTool` is using `interactiveProgress` to decide
whether to wire `onProgress`, which changes execution flow instead of just
disabling the spinner. Update the `isInteractive`/options handling in
`projects/cli/src/utils.ts` so `onProgress` is still passed through whenever the
async path should be used, even when `interactiveProgress` is false. In
`projects/cli/src/index.ts` and the `ManagedToolMethod` call site, keep a
callback attached and make it a no-op or logger-only implementation when
progress UI is off, so `projects/internals/tools/src/cli/utils.ts` does not fall
back to `runSync()`.
- Around line 182-187: The debug token usage display in utils.ts uses the raw
pct value for the “remaining” text, which can go negative when formattedResult
exceeds MAX_CONTEXT_TOKENS. In the debug block inside the tool/start/end check,
clamp pct to the 0–100 range before using it for the remaining calculation,
while still passing the clamped value to progressBar() so the output stays
consistent.
In `@projects/internals/tools/src/api/utils.ts`:
- Line 171: Normalize the injected registry base before it is used to build
package fetch URLs, since getElementsEnvValues() currently only trims whitespace
and leaves trailing slashes intact. Update the NPM_REGISTRY_URL handling in
utils.ts so the composed endpoint for `@nvidia-elements/core/latest` does not end
up with a double slash when ELEMENTS_REGISTRY_URL already ends with "/". Use the
existing getElementsEnvValues() and NPM_REGISTRY_URL symbols to locate the
parsing/composition logic and strip any trailing slash from the registry base
before concatenation.
In `@projects/internals/tools/src/playground/utils.ts`:
- Line 13: Normalize the injected ESM CDN base before it is used by
createImportMap(), since ELEMENTS_ESM_CDN_BASE_URL may include a trailing slash
and that combines badly with package paths that already start with a slash.
Update the shared constant in utils.ts, or the common env helper that feeds it,
so any trailing slash is stripped once and all custom CDN hosts remain valid
when building import maps.
In `@projects/internals/vite/src/configs/env.js`:
- Around line 20-35: The env mapping in getElementsEnv() is missing the
__ELEMENTS_ASSETS_CDN_BASE_URL__ define even though getElementsEnvValues()
already reads ELEMENTS_ASSETS_CDN_BASE_URL. Update getElementsEnv() to emit that
constant alongside the other Vite defines so callers like animated-build.ts
receive the assets CDN base URL correctly.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: d1c89635-2724-4e0f-b9bf-ea5e4204072c
📒 Files selected for processing (21)
projects/cli/scripts/write-install-manifest.mjsprojects/cli/src/index.tsprojects/cli/src/install.test.tsprojects/cli/src/install.tsprojects/cli/src/update.test.tsprojects/cli/src/update.tsprojects/cli/src/utils.test.tsprojects/cli/src/utils.tsprojects/cli/vite.config.tsprojects/internals/tools/src/api/utils.tsprojects/internals/tools/src/cli/utils.test.tsprojects/internals/tools/src/cli/utils.tsprojects/internals/tools/src/playground/utils.test.tsprojects/internals/tools/src/playground/utils.tsprojects/internals/vite/src/configs/build.jsprojects/internals/vite/src/configs/build.node.jsprojects/internals/vite/src/configs/env.jsprojects/internals/vite/src/configs/test.jsprojects/internals/vite/src/configs/test.node.jsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.ts
💤 Files with no reviewable changes (1)
- projects/site/src/_11ty/utils/env.test.ts
Comment on lines
+47
to
+52
| export async function runAsyncTool( | ||
| args: Record<string, unknown>, | ||
| fn: ManagedToolMethod<unknown>, | ||
| options: RunAsyncToolOptions = {} | ||
| ) { | ||
| const isInteractive = (options.interactiveProgress ?? true) && !args.start && !args.log && !process.env.CI; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't use interactiveProgress to suppress onProgress wiring.
interactiveProgress: false is changing execution semantics, not just the UI. projects/internals/tools/src/cli/utils.ts falls back to runSync() when the callback is missing, so the new upgrade path in projects/cli/src/index.ts now drops the async spawn()/stderr-capture path simply by disabling the spinner. Keep passing an onProgress callback when interactive output is off; make it a no-op or plain logger if you do not want spinner updates.
🤖 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 `@projects/cli/src/utils.ts` around lines 47 - 52, `runAsyncTool` is using
`interactiveProgress` to decide whether to wire `onProgress`, which changes
execution flow instead of just disabling the spinner. Update the
`isInteractive`/options handling in `projects/cli/src/utils.ts` so `onProgress`
is still passed through whenever the async path should be used, even when
`interactiveProgress` is false. In `projects/cli/src/index.ts` and the
`ManagedToolMethod` call site, keep a callback attached and make it a no-op or
logger-only implementation when progress UI is off, so
`projects/internals/tools/src/cli/utils.ts` does not fall back to `runSync()`.
Comment on lines
+182
to
+187
| if (tool && start !== undefined && end !== undefined && isDebug()) { | ||
| const tokens = formattedResult.length / 4; | ||
| const pct = (tokens / MAX_CONTEXT_TOKENS) * 100; | ||
| formattedResult += `[debug]\n[command]: ${tool.metadata.command}`; | ||
| formattedResult += `\n[execution time]: ${((end - start) / 1000).toFixed(2)} seconds`; | ||
| formattedResult += `\n[token usage]: ${progressBar(pct)} ${tokens.toLocaleString()} / ${MAX_CONTEXT_TOKENS.toLocaleString()} (${(100 - pct).toFixed(1)}% remaining)`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp the displayed token percentage before computing “remaining”.
progressBar() clamps over-100 values, but the trailing text uses the raw percentage. Once debug output exceeds MAX_CONTEXT_TOKENS, this prints a negative remaining percentage.
Suggested fix
- const pct = (tokens / MAX_CONTEXT_TOKENS) * 100;
+ const pct = Math.max(0, Math.min(100, (tokens / MAX_CONTEXT_TOKENS) * 100));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (tool && start !== undefined && end !== undefined && isDebug()) { | |
| const tokens = formattedResult.length / 4; | |
| const pct = (tokens / MAX_CONTEXT_TOKENS) * 100; | |
| formattedResult += `[debug]\n[command]: ${tool.metadata.command}`; | |
| formattedResult += `\n[execution time]: ${((end - start) / 1000).toFixed(2)} seconds`; | |
| formattedResult += `\n[token usage]: ${progressBar(pct)} ${tokens.toLocaleString()} / ${MAX_CONTEXT_TOKENS.toLocaleString()} (${(100 - pct).toFixed(1)}% remaining)`; | |
| if (tool && start !== undefined && end !== undefined && isDebug()) { | |
| const tokens = formattedResult.length / 4; | |
| const pct = Math.max(0, Math.min(100, (tokens / MAX_CONTEXT_TOKENS) * 100)); | |
| formattedResult += `[debug]\n[command]: ${tool.metadata.command}`; | |
| formattedResult += `\n[execution time]: ${((end - start) / 1000).toFixed(2)} seconds`; | |
| formattedResult += `\n[token usage]: ${progressBar(pct)} ${tokens.toLocaleString()} / ${MAX_CONTEXT_TOKENS.toLocaleString()} (${(100 - pct).toFixed(1)}% remaining)`; |
🤖 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 `@projects/cli/src/utils.ts` around lines 182 - 187, The debug token usage
display in utils.ts uses the raw pct value for the “remaining” text, which can
go negative when formattedResult exceeds MAX_CONTEXT_TOKENS. In the debug block
inside the tool/start/end check, clamp pct to the 0–100 range before using it
for the remaining calculation, while still passing the clamped value to
progressBar() so the output stays consistent.
| } | ||
|
|
||
| const NPM_REGISTRY_URL = 'https://registry.npmjs.org'; | ||
| const NPM_REGISTRY_URL = __ELEMENTS_REGISTRY_URL__; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Normalize the injected registry base before composing fetch URLs.
getElementsEnvValues() only trims whitespace, so a common setting like ELEMENTS_REGISTRY_URL=https://registry.npmjs.org/ becomes https://registry.npmjs.org//@nvidia-elements/core/latest here. That makes the new configurable registry endpoint brittle for trailing-slash inputs.
Proposed fix
-const NPM_REGISTRY_URL = __ELEMENTS_REGISTRY_URL__;
+const NPM_REGISTRY_URL = __ELEMENTS_REGISTRY_URL__.replace(/\/+$/, '');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const NPM_REGISTRY_URL = __ELEMENTS_REGISTRY_URL__; | |
| const NPM_REGISTRY_URL = __ELEMENTS_REGISTRY_URL__.replace(/\/+$/, ''); |
🤖 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 `@projects/internals/tools/src/api/utils.ts` at line 171, Normalize the
injected registry base before it is used to build package fetch URLs, since
getElementsEnvValues() currently only trims whitespace and leaves trailing
slashes intact. Update the NPM_REGISTRY_URL handling in utils.ts so the composed
endpoint for `@nvidia-elements/core/latest` does not end up with a double slash
when ELEMENTS_REGISTRY_URL already ends with "/". Use the existing
getElementsEnvValues() and NPM_REGISTRY_URL symbols to locate the
parsing/composition logic and strip any trailing slash from the registry base
before concatenation.
|
|
||
| const ELEMENTS_PLAYGROUND_BASE_URL = process.env.ELEMENTS_PLAYGROUND_BASE_URL ?? ''; | ||
| const ELEMENTS_ESM_CDN_BASE_URL = process.env.ELEMENTS_ESM_CDN_BASE_URL ?? ''; | ||
| const ELEMENTS_ESM_CDN_BASE_URL = __ELEMENTS_ESM_CDN_BASE_URL__; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Strip trailing slashes from the injected ESM CDN base.
createImportMap() appends package paths with a leading /, so ELEMENTS_ESM_CDN_BASE_URL=https://esm.sh/ generates https://esm.sh//@nvidia-elements/core@latest. Normalize the base here (or once in the shared env helper) so custom CDN hosts stay valid.
Proposed fix
-const ELEMENTS_ESM_CDN_BASE_URL = __ELEMENTS_ESM_CDN_BASE_URL__;
+const ELEMENTS_ESM_CDN_BASE_URL = __ELEMENTS_ESM_CDN_BASE_URL__.replace(/\/+$/, '');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const ELEMENTS_ESM_CDN_BASE_URL = __ELEMENTS_ESM_CDN_BASE_URL__; | |
| const ELEMENTS_ESM_CDN_BASE_URL = __ELEMENTS_ESM_CDN_BASE_URL__.replace(/\/+$/, ''); |
🤖 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 `@projects/internals/tools/src/playground/utils.ts` at line 13, Normalize the
injected ESM CDN base before it is used by createImportMap(), since
ELEMENTS_ESM_CDN_BASE_URL may include a trailing slash and that combines badly
with package paths that already start with a slash. Update the shared constant
in utils.ts, or the common env helper that feeds it, so any trailing slash is
stripped once and all custom CDN hosts remain valid when building import maps.
Comment on lines
+20
to
+35
| ELEMENTS_PLAYGROUND_BASE_URL: getEnvValue('ELEMENTS_PLAYGROUND_BASE_URL'), | ||
| ELEMENTS_ASSETS_CDN_BASE_URL: getEnvValue('ELEMENTS_ASSETS_CDN_BASE_URL') | ||
| }; | ||
| } | ||
|
|
||
| export function getElementsEnv() { | ||
| const env = getElementsEnvValues(); | ||
| return { | ||
| __ELEMENTS_REPO_BASE_URL__: JSON.stringify(env.ELEMENTS_REPO_BASE_URL), | ||
| __ELEMENTS_PAGES_BASE_URL__: JSON.stringify(env.ELEMENTS_PAGES_BASE_URL), | ||
| __ELEMENTS_REGISTRY_URL__: JSON.stringify(env.ELEMENTS_REGISTRY_URL), | ||
| __ELEMENTS_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_CDN_BASE_URL), | ||
| __ELEMENTS_ESM_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_ESM_CDN_BASE_URL), | ||
| __ELEMENTS_SITE_URL__: JSON.stringify(env.ELEMENTS_SITE_URL), | ||
| __ELEMENTS_PLAYGROUND_BASE_URL__: JSON.stringify(env.ELEMENTS_PLAYGROUND_BASE_URL) | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the missing assets CDN define.
ELEMENTS_ASSETS_CDN_BASE_URL is read on Line 21, but getElementsEnv() never emits __ELEMENTS_ASSETS_CDN_BASE_URL__. Since the other Vite configs now delegate entirely to this helper, consumers like projects/site/src/_internal/animated-build/animated-build.ts will never receive that constant and will fail to build/resolve asset URLs correctly.
Proposed fix
export function getElementsEnv() {
const env = getElementsEnvValues();
return {
__ELEMENTS_REPO_BASE_URL__: JSON.stringify(env.ELEMENTS_REPO_BASE_URL),
__ELEMENTS_PAGES_BASE_URL__: JSON.stringify(env.ELEMENTS_PAGES_BASE_URL),
__ELEMENTS_REGISTRY_URL__: JSON.stringify(env.ELEMENTS_REGISTRY_URL),
__ELEMENTS_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_CDN_BASE_URL),
__ELEMENTS_ESM_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_ESM_CDN_BASE_URL),
__ELEMENTS_SITE_URL__: JSON.stringify(env.ELEMENTS_SITE_URL),
- __ELEMENTS_PLAYGROUND_BASE_URL__: JSON.stringify(env.ELEMENTS_PLAYGROUND_BASE_URL)
+ __ELEMENTS_PLAYGROUND_BASE_URL__: JSON.stringify(env.ELEMENTS_PLAYGROUND_BASE_URL),
+ __ELEMENTS_ASSETS_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_ASSETS_CDN_BASE_URL)
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ELEMENTS_PLAYGROUND_BASE_URL: getEnvValue('ELEMENTS_PLAYGROUND_BASE_URL'), | |
| ELEMENTS_ASSETS_CDN_BASE_URL: getEnvValue('ELEMENTS_ASSETS_CDN_BASE_URL') | |
| }; | |
| } | |
| export function getElementsEnv() { | |
| const env = getElementsEnvValues(); | |
| return { | |
| __ELEMENTS_REPO_BASE_URL__: JSON.stringify(env.ELEMENTS_REPO_BASE_URL), | |
| __ELEMENTS_PAGES_BASE_URL__: JSON.stringify(env.ELEMENTS_PAGES_BASE_URL), | |
| __ELEMENTS_REGISTRY_URL__: JSON.stringify(env.ELEMENTS_REGISTRY_URL), | |
| __ELEMENTS_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_CDN_BASE_URL), | |
| __ELEMENTS_ESM_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_ESM_CDN_BASE_URL), | |
| __ELEMENTS_SITE_URL__: JSON.stringify(env.ELEMENTS_SITE_URL), | |
| __ELEMENTS_PLAYGROUND_BASE_URL__: JSON.stringify(env.ELEMENTS_PLAYGROUND_BASE_URL) | |
| }; | |
| ELEMENTS_PLAYGROUND_BASE_URL: getEnvValue('ELEMENTS_PLAYGROUND_BASE_URL'), | |
| ELEMENTS_ASSETS_CDN_BASE_URL: getEnvValue('ELEMENTS_ASSETS_CDN_BASE_URL') | |
| }; | |
| } | |
| export function getElementsEnv() { | |
| const env = getElementsEnvValues(); | |
| return { | |
| __ELEMENTS_REPO_BASE_URL__: JSON.stringify(env.ELEMENTS_REPO_BASE_URL), | |
| __ELEMENTS_PAGES_BASE_URL__: JSON.stringify(env.ELEMENTS_PAGES_BASE_URL), | |
| __ELEMENTS_REGISTRY_URL__: JSON.stringify(env.ELEMENTS_REGISTRY_URL), | |
| __ELEMENTS_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_CDN_BASE_URL), | |
| __ELEMENTS_ESM_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_ESM_CDN_BASE_URL), | |
| __ELEMENTS_SITE_URL__: JSON.stringify(env.ELEMENTS_SITE_URL), | |
| __ELEMENTS_PLAYGROUND_BASE_URL__: JSON.stringify(env.ELEMENTS_PLAYGROUND_BASE_URL), | |
| __ELEMENTS_ASSETS_CDN_BASE_URL__: JSON.stringify(env.ELEMENTS_ASSETS_CDN_BASE_URL) | |
| }; |
🤖 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 `@projects/internals/vite/src/configs/env.js` around lines 20 - 35, The env
mapping in getElementsEnv() is missing the __ELEMENTS_ASSETS_CDN_BASE_URL__
define even though getElementsEnvValues() already reads
ELEMENTS_ASSETS_CDN_BASE_URL. Update getElementsEnv() to emit that constant
alongside the other Vite defines so callers like animated-build.ts receive the
assets CDN base URL correctly.
42deb6f to
10805ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@projects/cli/src/index.ts`:
- Around line 101-104: The command synopsis builder in index.ts is incorrectly
hiding all optional args when `hasVariadicArg` is true, which removes flag-style
options like `--format` from help output. Update the `command` construction
logic so the variadic path still appends optional flag arguments from
`optionalArgs` while excluding only extra positional placeholders, keeping
`tool.metadata.command` help text consistent for commands such as `api.get`.
In `@projects/internals/tools/src/project/health.ts`:
- Around line 190-212: The dependency checks in checkDependencies and
getVersionHealth are using different package filters, which can leave unknown
`@nvidia-elements` package names in the version map and cause getVersionStatus to
fail on undefined data. Update the logic around currentVersions,
allDependencies, and getVersionHealth so both paths apply the same known-package
filter (or share one helper) and skip any dependency names that are not present
in currentVersions before building or reading versions.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 21e6c348-57a9-4ec3-8f6e-2acba88121a3
📒 Files selected for processing (27)
projects/cli/README.mdprojects/cli/scripts/write-install-manifest.mjsprojects/cli/src/index.test.tsprojects/cli/src/index.tsprojects/cli/src/install.test.tsprojects/cli/src/install.tsprojects/cli/src/update.test.tsprojects/cli/src/update.tsprojects/cli/src/utils.test.tsprojects/cli/src/utils.tsprojects/cli/vite.config.tsprojects/internals/tools/src/api/utils.test.tsprojects/internals/tools/src/api/utils.tsprojects/internals/tools/src/cli/utils.test.tsprojects/internals/tools/src/cli/utils.tsprojects/internals/tools/src/playground/utils.test.tsprojects/internals/tools/src/playground/utils.tsprojects/internals/tools/src/project/health.test.tsprojects/internals/tools/src/project/health.tsprojects/internals/vite/src/configs/build.jsprojects/internals/vite/src/configs/build.node.jsprojects/internals/vite/src/configs/env.jsprojects/internals/vite/src/configs/test.jsprojects/internals/vite/src/configs/test.node.jsprojects/site/src/_11ty/utils/env.jsprojects/site/src/_11ty/utils/env.test.tsprojects/site/src/docs/cli/index.md
💤 Files with no reviewable changes (1)
- projects/site/src/_11ty/utils/env.test.ts
Comment on lines
+101
to
+104
| const command = `${tool.metadata.command} ${[ | ||
| ...positionalArgs, | ||
| ...(hasVariadicArg ? [] : optionalArgs.map(key => `[${key}]`)) | ||
| ].join(' ')}`.trim(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- index.ts around command generation ---\n'
sed -n '80,130p' projects/cli/src/index.ts
printf '\n--- index.test.ts around help assertions ---\n'
sed -n '1,220p' projects/cli/src/index.test.ts
printf '\n--- docs references for api.get / --format ---\n'
rg -n "api\.get|--format|<names\.\.>" projects -g '!**/node_modules/**'Repository: NVIDIA/elements
Length of output: 244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- index.ts around command generation ---'
sed -n '80,130p' projects/cli/src/index.ts
echo
echo '--- index.test.ts around help assertions ---'
sed -n '1,220p' projects/cli/src/index.test.ts
echo
echo '--- docs references for api.get / --format ---'
rg -n "api\.get|--format|<names\.\.>" projects -g '!**/node_modules/**'Repository: NVIDIA/elements
Length of output: 12946
Keep --format in variadic command usage.
The variadic branch drops every optional arg from the displayed synopsis, so nve --help no longer advertises api.get’s --format flag. Keep flag-style options visible, e.g. nve api.get <names..> [--format <format>].
🤖 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 `@projects/cli/src/index.ts` around lines 101 - 104, The command synopsis
builder in index.ts is incorrectly hiding all optional args when
`hasVariadicArg` is true, which removes flag-style options like `--format` from
help output. Update the `command` construction logic so the variadic path still
appends optional flag arguments from `optionalArgs` while excluding only extra
positional placeholders, keeping `tool.metadata.command` help text consistent
for commands such as `api.get`.
Comment on lines
190
to
+212
| const elementPackages = Object.keys(currentVersions); | ||
| const allDependencies = [...dependencies, ...devDependencies, ...peerDependencies]; | ||
| let status: 'success' | 'danger' | 'warning' = 'success'; | ||
| let message = '@nvidia-elements packages are up to date'; | ||
|
|
||
| if (!allDependencies.some(dependency => elementPackages.includes(dependency))) { | ||
| status = 'warning'; | ||
| message = 'No @nvidia-elements packages found in the project'; | ||
| return { | ||
| versions: {}, | ||
| status: 'warning' as const, | ||
| message: 'No @nvidia-elements packages found in the project' | ||
| }; | ||
| } | ||
|
|
||
| const unavailablePackages = Object.entries(currentVersions) | ||
| .filter(([packageName, version]) => allDependencies.includes(packageName) && version === '0.0.0') | ||
| .map(([packageName]) => packageName); | ||
| if (unavailablePackages.length > 0) { | ||
| return { | ||
| versions: {}, | ||
| status: 'danger' as const, | ||
| message: `Could not verify latest versions for ${unavailablePackages.join(', ')}` | ||
| }; | ||
| } | ||
|
|
||
| const versions = await getVersionHealth(packageJson, currentVersions); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== checkDependencies / getVersionHealth =="
nl -ba projects/internals/tools/src/project/health.ts | sed -n '29,225p'
echo
echo "== ElementVersions contract =="
nl -ba projects/internals/tools/src/api/utils.ts | sed -n '159,210p'
echo
echo "== Existing tests touching checkDependencies =="
rg -n "checkDependencies\\(" projects/internals/tools/src/project/health.test.ts
echo
echo "== Any test covering an unknown `@nvidia-elements` package? =="
rg -n "`@nvidia-elements/`" projects/internals/tools/src/project/health.test.tsRepository: NVIDIA/elements
Length of output: 236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== projects/internals/tools/src/project/health.ts =="
sed -n '1,260p' projects/internals/tools/src/project/health.ts
echo
echo "== projects/internals/tools/src/api/utils.ts =="
sed -n '140,220p' projects/internals/tools/src/api/utils.ts
echo
echo "== projects/internals/tools/src/project/health.test.ts =="
sed -n '1,260p' projects/internals/tools/src/project/health.test.ts
echo
echo "== Search for any callers or related package filtering =="
grep -RIn --exclude-dir=node_modules --exclude-dir=.git -E "getVersionHealth|checkDependencies|`@nvidia-elements/`" projects/internals/tools/src | head -n 200Repository: NVIDIA/elements
Length of output: 45742
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path('projects/internals/tools/src/project/health.test.ts')
lines = path.read_text().splitlines()
for start, end in [(150, 280), (280, 380)]:
print(f"== {path} lines {start}-{end} ==")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
print()
PYRepository: NVIDIA/elements
Length of output: 9431
Align the dependency filter with getVersionHealth(). checkDependencies() only counts packages already present in currentVersions, but getVersionHealth() still iterates any @nvidia-elements/* dependency. If a project has one known Elements package and one unknown one, versions[packageName]! can be undefined and getVersionStatus() will throw. Skip unknown package names in both places or share the same filter.
🤖 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 `@projects/internals/tools/src/project/health.ts` around lines 190 - 212, The
dependency checks in checkDependencies and getVersionHealth are using different
package filters, which can leave unknown `@nvidia-elements` package names in the
version map and cause getVersionStatus to fail on undefined data. Update the
logic around currentVersions, allDependencies, and getVersionHealth so both
paths apply the same known-package filter (or share one helper) and skip any
dependency names that are not present in currentVersions before building or
reading versions.
- Added support for versioning output files in the build process, ensuring that the correct version is written to bundled files. - Introduced a new utility function to replace bundled constants in the output files, improving maintainability. - Updated the installation script to utilize the embedded build SHA and version, enhancing the manifest generation. - Refactored error handling in the update check to ensure proper feedback when the SHA is missing. Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-cli-version-updater
branch
from
10805ab to
0f578a5
Compare
github-actions
Bot
added
the
dependencies
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
1 participant
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 by CodeRabbit
sha,version, and per-platform checksums derived from bundled build outputs.SKILL.mdwithout prompting (including CI), reporting “Installed” vs “Updated”.nve api.getusage and command signature.