feat(skills): add dependency upgrade audit workflow#6740
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a contributor skill for dependency upgrades, supporting audit and release-ledger references, a hardened Git/GitHub ledger collector, skills-catalog registration, agent configuration, and Vitest coverage for policy, integrity, security, and collector behavior. ChangesDependency upgrade contributor skill
Estimated code review effort: 5 (Critical) | ~100 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Contributor
participant CollectorCLI
participant ReleaseLedgerCollector
participant GitRepository
participant GitHubAPI
Contributor->>CollectorCLI: provide repository and release refs
CollectorCLI->>ReleaseLedgerCollector: run collection command
ReleaseLedgerCollector->>GitRepository: validate history and collect local evidence
GitRepository-->>ReleaseLedgerCollector: return tags, ancestry, commits, and paths
ReleaseLedgerCollector->>GitHubAPI: validate remote identities and publications
GitHubAPI-->>ReleaseLedgerCollector: return release and tag evidence
ReleaseLedgerCollector-->>Contributor: emit deterministic JSON release ledger
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 79%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated review. Required findings need action before merge. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
@.agents/skills/nemoclaw-contributor-update-dependencies/scripts/collect-release-ledger.py:
- Around line 226-252: Preserve metadata for an explicitly targeted prerelease
in the candidate-building flow: when a tag resolves to target_sha and matches
args.to_ref, do not exclude it via the include_prereleases filter. Ensure
endpoint_for_tag supplies the target endpoint’s real version and tagKind, while
retaining the existing prerelease filtering for other tags and the bare fallback
for untagged targets.
🪄 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: e6df73da-10ad-4814-be8e-4ed747973b72
📒 Files selected for processing (7)
.agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md.agents/skills/nemoclaw-contributor-update-dependencies/agents/openai.yaml.agents/skills/nemoclaw-contributor-update-dependencies/references/contract-audit.md.agents/skills/nemoclaw-contributor-update-dependencies/references/release-ledger.md.agents/skills/nemoclaw-contributor-update-dependencies/scripts/collect-release-ledger.py.agents/skills/nemoclaw-skills-guide/SKILL.mdtest/dependency-upgrade-skill.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
cjagwani
left a comment
There was a problem hiding this comment.
Reviewed exact head 9b15830. I cannot approve this authenticated collector yet:
-
High — establish an agent/executable trust boundary. SKILL.md:113-130 ingests upstream-controlled release notes, commits, diffs, source, and tests, while SKILL.md:446-449 says to execute the checkout-local collector. Explicitly treat all upstream content as untrusted evidence, never instructions; verify this collector and every transitive execution surface against refreshed trusted origin/main before execution; resolve reviewed Python/Git/GH executables before consuming that content; and pass gh a minimal allowlisted environment. Add PATH-shim and prompt-injection regressions.
-
Medium — bound output volume, not only elapsed time. collect-release-ledger.py:191-200, 452-477, 576-622, 640-687, 909-913, and 1039-1043 can buffer complete Git histories/object/path inventories and paginated GitHub responses. Stream these sources, enforce explicit byte/record ceilings, terminate on excess, and fail closed with oversized-output tests.
-
Low/Medium — create the private ledger as mode 0600 at creation time. collect-release-ledger.py:1583-1586 uses Path.open('x'), which produces 0644 under a normal umask. Add an umask-independent permission test.
-
Review gate — CodeRabbit is paused and only reviewed c1a242e, 18 first-parent commits behind this head. Require a fresh current-head CodeRabbit review before approval.
The nine-category security pass found no hardcoded secret, shell injection, SSRF, auth bypass, unsafe deserialization, or unauthorized GitHub mutation; the items above are the remaining approval blockers.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…de-skill # Conflicts: # .agents/skills/nemoclaw-skills-guide/SKILL.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/dependency-upgrade-skill.test.ts (1)
332-413: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTest public contracts instead of exact prose and private parser internals.
Keep only explicitly contractual wording checks. Replace the remaining phrase assertions with structural/validator tests, and test malformed identifiers through the collector CLI with a fake
gitthat proves no Git invocation occurred.As per path instructions, prefer observable outcomes through the public boundary over source-text or private-shape assertions.
Also applies to: 1319-1338
🤖 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/dependency-upgrade-skill.test.ts` around lines 332 - 413, Refactor the dependency-upgrade skill tests to assert only explicit public contracts, replacing non-contractual exact-prose and private-parser-shape checks with structural or validator behavior tests. Exercise malformed identifiers through the collector CLI, using a fake git executable to verify validation rejects them without invoking Git. Apply the same approach to the related assertions around the additional referenced test range, preserving observable public-boundary outcomes.Source: Path instructions
test/dependency-upgrade-skill-security.test.ts (1)
354-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise malformed GitHub payloads through the CLI boundary.
The
runpyprobe calls private validators directly, so it still passes ifcollect()stops routing GitHub payloads through them. Return the malformed URLs from a fakegh, invokerunCollector(), and assert the CLI fails closed.As per path instructions, prefer observable outcomes through the public boundary over private-shape assertions.
🤖 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/dependency-upgrade-skill-security.test.ts` around lines 354 - 382, Replace the direct runpy calls to private validators in the test “rejects noncanonical URLs and release URLs bound to another tag” with a fake gh executable that returns the malformed URL and mismatched release payload. Invoke runCollector() through the CLI boundary and assert it fails closed with a nonzero status, preserving coverage for both malformed URL cases and the release/tag mismatch.Source: Path instructions
🤖 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
@.agents/skills/nemoclaw-contributor-update-dependencies/scripts/collect-release-ledger.py:
- Around line 1368-1375: Update the remote-tag loop around
require_local_commit_object and the ancestry checks to evaluate whether
commit_sha is within the start_sha..target_sha range before requiring its local
object. Skip absent local commits that are outside the range, while retaining
exact local tag matching via require_exact_local_tag for in-range commits whose
objects are available.
- Around line 1876-1901: Update the file-output branch in main to create a
mode-0600 temporary file in the destination directory, write the payload, and
fsync it before publication. Atomically publish the completed temporary file to
the requested output path using no-replace semantics, and ensure cleanup removes
the temporary file on write, fsync, or publish failure without leaving partial
output at the final path.
- Around line 1285-1328: Apply the existing record and endpoint ceilings before
any per-tag expansion: batch local ref metadata, count SemVer tags during
collection, and reject excess tags before invoking per-tag subprocesses,
github_tag_identity_from_root, or ancestry checks. Update
github_semver_tag_inventory to enforce the SemVer-tag ceiling while processing
paginated refs, before remote tag peeling begins, and preserve duplicate/non-tag
validation.
---
Nitpick comments:
In `@test/dependency-upgrade-skill-security.test.ts`:
- Around line 354-382: Replace the direct runpy calls to private validators in
the test “rejects noncanonical URLs and release URLs bound to another tag” with
a fake gh executable that returns the malformed URL and mismatched release
payload. Invoke runCollector() through the CLI boundary and assert it fails
closed with a nonzero status, preserving coverage for both malformed URL cases
and the release/tag mismatch.
In `@test/dependency-upgrade-skill.test.ts`:
- Around line 332-413: Refactor the dependency-upgrade skill tests to assert
only explicit public contracts, replacing non-contractual exact-prose and
private-parser-shape checks with structural or validator behavior tests.
Exercise malformed identifiers through the collector CLI, using a fake git
executable to verify validation rejects them without invoking Git. Apply the
same approach to the related assertions around the additional referenced test
range, preserving observable public-boundary outcomes.
🪄 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: 9d541be5-c9fc-4039-905c-d75f4db40511
📒 Files selected for processing (7)
.agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md.agents/skills/nemoclaw-contributor-update-dependencies/references/contract-audit.md.agents/skills/nemoclaw-contributor-update-dependencies/references/release-ledger.md.agents/skills/nemoclaw-contributor-update-dependencies/scripts/collect-release-ledger.py.agents/skills/nemoclaw-skills-guide/SKILL.mdtest/dependency-upgrade-skill-security.test.tstest/dependency-upgrade-skill.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .agents/skills/nemoclaw-contributor-update-dependencies/references/contract-audit.md
- .agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
cjagwani
left a comment
There was a problem hiding this comment.
Approving exact head 4d77f100f5fe2afd221a1d15098dfbba444caa72.
All required checks are green; every commit is GitHub-verified; both PR Advisor models report no findings; the exact-head CodeRabbit incremental review has no actionable comments; and all 12 review threads are resolved. Independent re-audit confirms the tag ceiling, absent-object handling, and atomic no-replace publication blockers are fixed. Focused validation passed 51/51 tests plus adversarial mode, symlink, no-replace, and link-failure probes.
The parent-directory fsync observation is non-blocking hardening and does not recreate partial-final publication. This is ready for human merge follow-up.
<!-- markdownlint-disable MD041 --> ## Summary Release-prep documentation for v0.0.82 now summarizes user-facing changes merged since v0.0.81. It also closes stale wording in the stopped-sandbox backup, snapshot-clone, Ollama selection, and custom-policy authoring guidance. ## Changes - Add the `v0.0.82` section to `docs/about/release-notes.mdx` with links to the focused user guides. - Document that snapshot clones receive a destination-owned dashboard port before destructive replacement begins. - Align `backup-all` guidance with eligible stopped Docker-driver sandboxes that NemoClaw starts temporarily. - Describe the running and stopped Ollama menu states without claiming one fixed label. - Document runtime rejection of catch-all hosts in custom policy files. ### Source summary - [#6748](#6748) -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/lifecycle.mdx`, and `docs/reference/commands.mdx`: Summarize non-destructive sandbox `stop` and `start` commands. - [#6723](#6723) -> `docs/about/release-notes.mdx`, `docs/manage-sandboxes/backup-restore.mdx`, and `docs/reference/commands.mdx`: Record temporary startup and cleanup for eligible stopped-sandbox backups. - [#6749](#6749) -> `docs/about/release-notes.mdx` and `docs/manage-sandboxes/backup-restore.mdx`: Document destination-owned dashboard ports for snapshot clones. - [#6764](#6764) -> `docs/about/release-notes.mdx`: Summarize installer handling of route-only onboarding placeholders. - [#6771](#6771) -> `docs/about/release-notes.mdx`, `docs/inference/set-up-vllm.mdx`, `docs/inference/choose-inference-provider.mdx`, `docs/reference/commands.mdx`, and `docs/reference/platform-support.mdx`: Summarize managed-vLLM storage gates, immutable image digests, and the explicit override boundary. - [#6759](#6759) -> `docs/about/release-notes.mdx`: Record early, actionable OpenShell gateway-port conflict diagnostics. - [#6753](#6753) -> `docs/about/release-notes.mdx` and `docs/inference/set-up-ollama.mdx`: Document truthful running and stopped Ollama menu states. - [#6776](#6776) -> `docs/about/release-notes.mdx`: Summarize proxy-independent loopback readiness checks. - [#6769](#6769) -> `docs/about/release-notes.mdx`: Record compatible endpoint and agent guidance when Chat Completions is unavailable. - [#6730](#6730) -> `docs/about/release-notes.mdx`: Summarize bounded reuse of an eligible successful Chat Completions check. - [#6768](#6768) -> `docs/about/release-notes.mdx`: Record route-reservation repair during resumed onboarding. - [#6742](#6742) -> `docs/about/release-notes.mdx`: Summarize pre-mutation resolution of secret-free sandbox create intent. - [#6721](#6721) -> `docs/about/release-notes.mdx` and `docs/get-started/quickstart-langchain-deepagents-code.mdx`: Record bounded cleanup of completed managed Deep Agents headless sessions. - [#6731](#6731) -> `docs/about/release-notes.mdx` and `docs/network-policy/customize-network-policy.mdx`: Document runtime rejection of catch-all custom-policy destinations. - [#6729](#6729) -> `docs/about/release-notes.mdx` and `docs/get-started/prerequisites.mdx`: Record the Node.js 22.19 minimum. - [#6735](#6735) -> `docs/about/release-notes.mdx` and `docs/reference/platform-support.mdx`: Summarize the Ubuntu 26.04 userspace contract without claiming pending host or live validation. - [#6775](#6775) -> `docs/about/release-notes.mdx` and `docs/resources/community-contributions.mdx`: Route independent solutions outside canonical supported-product documentation. - [#6740](#6740) -> `docs/about/release-notes.mdx`: Summarize the semantic dependency-upgrade contributor workflow. - [#6777](#6777) -> `docs/about/release-notes.mdx` and `docs/CONTRIBUTING.md`: Summarize the route-safe documentation-refactor workflow. - [#6741](#6741) -> `docs/about/release-notes.mdx` and `docs/security/openclaw-2026.6.10-dependency-review.md`: Summarize reviewed npm archive verification and audit enforcement. - [#6739](#6739) -> `docs/about/release-notes.mdx` and `docs/security/openclaw-2026.6.10-dependency-review.md`: Record the locked offline dependency graph for the managed OpenClaw WeChat runtime. - [#6737](#6737) -> `docs/about/release-notes.mdx`: Record removal of the messaging build plan from final OpenClaw and Hermes image environments. - [#6733](#6733) -> `docs/about/release-notes.mdx`: Summarize cached plugin dependency layers for source and blueprint rebuilds. ### Skipped from docs-skip - None. No commit or changed path in `v0.0.81..origin/main` matched `openclaw-sandbox-permissive.yaml` or `config-show`, and the drafted content contains none of the configured skip terms. ## 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This is a documentation-only release-prep update; behavior is protected by the merged source PRs, and the documentation build validates the changed routes and agent variants. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — tests are not applicable for this documentation-only change. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not run for this documentation-only change. - [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) — 0 errors; two pre-existing Fern warnings remain. - [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) — no new pages. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated release notes with improvements to sandbox recovery, onboarding, session management, policy validation, storage checks, and system requirements. * Clarified Ollama setup instructions and status labels. * Documented safer snapshot restoration, including dedicated ports and protection against destructive failures. * Expanded `backup-all` coverage to include eligible stopped sandboxes. * Added guidance rejecting broad or catch-all network destinations in custom policies. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Summary
Adds a contributor skill that treats dependency upgrades as release-by-release semantic migrations instead of version-only changes. It requires upstream source/test evidence, downstream contract tracing, concern-specific proofs, and a separate immutable-artifact audit before a bump can be called ready.
Changes
nemoclaw-contributor-update-dependencieswith a mandatory adjacent-release ledger, source-first contract audit, workaround/removal review, concern ledger, platform/runtime proof requirements, conditional-skip audit, and final-tag revalidation.NVIDIA/NemoClaw; dependency repositories, workflows, issues, and pull requests remain read-only.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration test/skills-frontmatter.test.ts test/dependency-upgrade-skill.test.ts(32 passed); skill validator passednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable; this adds one standalone contributor skill/collector and focused integration tests without changing runtime or the shared test harness.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit