Repository navigation
fix(ci): resolve Ubuntu mirrorlist for isolated Pi tools - #12837
Conversation
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe pinned Ubuntu package installer now creates a temporary APT sources file, replaces the expected mirror-file URI with the Ubuntu archive URI, and uses that file for APT operations. Contract tests cover source rewriting, unsupported source rejection, and use of the temporary file by both APT calls. ChangesPinned-package APT source isolation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: 🟡 Moderate · up to The updated installer has not yet been shown to unblock the CLI test shards. Verify the revised helper and required shard completion before treating the CI fix as complete. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit d38857f in the Show a line coverage summary of the most impacted files.
Updated |
|
CI disposition for exact head |
Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
One-time CI exception record for #11320Rebecca approved this one-time exception on October 8, 2026. This record was written after PR #12837 merged as Failed PR evidence: Exact-head PR CI run 37790162279 failed all 12 Candidate evidence: Manual Advisor run 37790303426 checked out the exact head and ran the revised helper on an Ubuntu runner. Pinned APT update and install both completed. All nine Advisor specialist artifacts reported zero findings, and the blocker gate passed. The focused workflow contract suite passed 69 tests. CodeRabbit completed its exact-head review with no actionable code comments. An eligible human approval was present before merge. Scope and recovery: This exception accepted only the missing pre-merge CLI shard evidence caused by the base-pinned CI action. It did not waive signed commits, automated review, human review, or unrelated failures. No agent admin bypass or ruleset change was used. Post-merge main CI run 37808065418 is now the acceptance gate: the revised helper must run and all 12 CLI shards must complete tests, especially shards 1, 5, 8, and 12. Issue #11320 has been reopened until that result is verified. If an affected shard fails, classify and repair the failure before closing the issue or recommending recovery for #12763 or #12764. |
## Outcome The independent E2E growth check can accept the approved budget for #12767 after this prerequisite policy merges into `main`. ## Reason Independent enforcement reads exceptions from the trusted base. A candidate-only entry cannot authorize its own budget change. ### Related issues Refs #7475. Prerequisite for #12767. ## Changes Add one exception bound to PR #12767 and both budget SHA-256 hashes. Preserve all existing exception entries. Remove the temporary entry after #12767 merges. The approved repair moves restoration validation into one builder within the scanned live source set. It preserves every restoration predicate, including plugin validation, and retains the seven-assertion reduction from #12608. The budget changes from `53c51c4229d387633165771730f8f1a8373f84e9f3303b2144280027a56e3441` to `1653f4c6c0292d2853727c39ddb36aa55361f749df3db32aa3847c3d4a7f30cb`. ## Verification - Existing growth parser and guardrail tests: 65 passed. - Production policy evaluator with the actual budget files: approved transition passes; wrong PR, changed base bytes, changed candidate bytes, and candidate-only policy are rejected. - CLI type check: passed with an 8 GiB Node heap. Default setup exhausted its default heap. - The diff changes only the exception policy and contains no secrets, API keys, or credentials. - Normal commit hooks: passed, including growth guardrails, repository checks, secret scan, and commitlint. - Final publication validation and CLI TypeScript: passed; plugin and JavaScript configuration checks legitimately skipped. All three published commits are GitHub Verified, including `3c4688d37d17f3a8e3aa96a933bb073924430ce2`. The previous candidate passed growth enforcement and CodeRabbit. Its CI failed before tests in the inherited Ubuntu mirrorlist installer. This candidate integrates the landed fix from #12837 and resolves the independent policy-entry conflict while preserving both entries. [CI run 37809262635](https://github.com/NVIDIA/NemoClaw/actions/runs/37809262635) passed for the integrated candidate, including all twelve CLI shards. Independent growth enforcement passed. No live E2E is required for this policy-only diff; all nine specialists concurred. ## Review notes [Rebecca approved this bounded relocation, budget transition, and prerequisite PR](#12767 (comment)). Her maintainer permission and the record were verified. This authorization is not a PR review approval or an Advisor waiver. CodeRabbit completed its exact-head review with no actionable findings. All nine Advisor specialists reported clear, and the blocker gate passed in [Advisor run 37811789238](https://github.com/NVIDIA/NemoClaw/actions/runs/37811789238). The policy PR changes no executable, live test, or budget file. Its parser continues to enforce PR identity and both hashes. The main repair remains local until this dependency lands. Local Docker and CLI exposure are not required for these policy checks. --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated internal validation records for an approved exception, including its review conditions, validity requirements, and verification details. No changes to restoration behavior or assertion counts are reflected in these records. * **Documentation** * Recorded that a previously approved assertion reduction remains in effect and noted when the exception is expected to be removed. These updates do not change any user-facing functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
## Outcome Refresh the exception for #12382 so its reviewed runtime assertions can pass the independent growth check after #12608 reduced main's gateway assertions. ## Reason #12608 merged before #12718 and changed the base budget bytes. The exception adopted by #12718 therefore matches neither the new base budget nor #12382's prepared integration. ### Related issues Refs #12382, #12718, #12608. ## Changes Replace only the two budget digests for #12382 and add a scoped explanation. Preserve every other exception and all enforcement code. | Budget | SHA-256 | |---|---| | Base | `53c51c4229d387633165771730f8f1a8373f84e9f3303b2144280027a56e3441` | | Runtime candidate | `8162408384d7a6b97ef0062042c73d4bc9486f54d2dfa6c0c5732e42d1060e2a` | The runtime transition retains native admin approval before survival/rebuild mutations and the plugin-install success check. It adds one unique assertion point and one generated probe condition over the reduced main budget. It adds no live scenario or matrix lane. ## Verification - Canonical `e2eAssertionBudgetGrowthViolations` consumer: accepts the proposed trusted-base transition; rejects a different PR, changed base bytes, changed candidate bytes, and candidate-only policy. - Assertion census: the prepared runtime source fits the proposed budget. - All 58 growth parser tests passed after resolving the shared-policy conflict. Existing census evidence remains valid because the budgets and runtime source did not change. - Signed commit and normal publication hooks passed, including plugin and checked-JavaScript type checks. - Reviewed the complete one-file diff. No secrets, API keys, or credentials. ## Review notes The branch incorporates #12837, which repairs the inherited Ubuntu mirrorlist installation failure. The branch also incorporates #12833 and preserves its #12767 exception. The #12382 budget digests are unchanged. The policy comment now cites the maintainer acceptance recorded below; no exception value or enforcement code changed. Signed commit `71535b2` passed normal commit and publication hooks. Fresh CI and Advisor checks for this comment-only update are pending. [Senthil approved the PR](#12840 (review)), confirming that the recorded acceptance resolves his earlier change request. Maintainer Prekshi Vyas (@prekshivyas) [accepted the exact replacement budget pair](#12840 (comment)), including one additional unique assertion point and one generated probe condition. Independent enforcement is not waived: adoption on main remains required before #12382 can use this exception. The approval records the exact replacement pair and its bounded assertion increase. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated internal CI records with the approved exception’s refreshed verification values. This does not change end-user functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## Outcome Pinned Ubuntu tools install when `RUNNER_TEMP` has private parent directories. APT package lists use a unique directory under `/tmp`, which its `_apt` sandbox can traverse without changing runner-directory permissions. ## Reason Changing only `RUNNER_TEMP` permissions cannot grant access through a private ancestor. A disposable Ubuntu AMD64 container reproduced the resulting mirror-list failure with the original helper. This change retains the Ubuntu mirrorlist resolution merged in #12837. That merge addresses the shared CI installation failure; this PR preserves directory access and private runner permissions alongside it. ### Related issues Refs #12382, #12840, #12778, #12837. ## Changes - Create the package-list directory under `/tmp` and retain its permissions, ownership, and cleanup. - Remove changes to `RUNNER_TEMP` permissions. Logs and the isolated Ubuntu source remain in the runner directory. - Make the workflow test fixture remove the temporary package-list directory. Assert that APT uses `/tmp`, runner permissions are not widened, and the directory is removed. Package pins, source restrictions, timeouts, retries, and failure propagation remain unchanged from main. ## Verification - `vitest run --project integration test/automation/pull-requests/pr-workflow-contract.test.ts`: all 69 tests passed after integrating #12837. - Real Ubuntu 24.04 AMD64 installation with the combined helper passed under a private parent directory. Installed `fd-find 9.0.0-1` and `ripgrep 14.1.0-1`. - Verified private directories retain mode `0700`, the system Ubuntu source is unchanged, temporary files are removed, and the disposable container is removed. - Regression mutations: moving lists back under `RUNNER_TEMP`, adding privileged permission widening, and omitting cleanup each fail the strengthened test. The complete 69-test workflow suite passes with the correct helper. - Earlier reproduction: the original helper exited 100; the directory repair passed under the same private ancestor. - Signed commit and normal publication hooks passed, including CLI type checking. - Reviewed the complete two-file diff. No secrets, API keys, or credentials. ## Review notes Self-review of commit `29e5dc95280829f591a38f42f8f707c7e0b41d30` covered the complete NVIDIA/NemoClaw diff against main `5f765a1318a2338114a1f6865de3113d9083b405`, including the sensitive `.github/actions/ci-install-pinned-ubuntu-packages.sh` path. The combined helper preserves #12837's source restrictions and passed the real Linux installation and cleanup checks. Advisor identified a regression-test gap; this commit repairs it without changing the installer. All current checks pass, including [core CI](https://github.com/NVIDIA/NemoClaw/actions/runs/37815724516), [image qualification](https://github.com/NVIDIA/NemoClaw/actions/runs/37815730289), and [all nine Advisor reviews](https://github.com/NVIDIA/NemoClaw/actions/runs/37820691834). The first CI attempt hit an inherited five-second vLLM unit-test timeout; one maintainer-approved failed-job rerun passed on the unchanged commit. All nine Advisor reports are clear and recommend no additional live E2E for this CI-helper change. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated CI package setup to store temporary Ubuntu package data under `/tmp` and clean it up, including when setup fails. The process no longer changes permissions on the runner’s temporary directory. * **Tests** * Added checks for temporary package-data cleanup after setup failures and verified that required paths and permissions are accessible during installation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
## Outcome Record the approved two-assertion exception for Slice 4 PR #12777 in trusted base policy. Once this prerequisite merges, independent enforcement can evaluate that exact budget transition without accepting policy supplied by the feature branch. ## Reason The independent growth check reads exceptions from main. The candidate-only record in #12777 cannot satisfy that check. ### Related issues Refs #12558. Prerequisite for #12777. ## Changes Add one PR-number-bound entry to `ci/e2e-assertion-growth-exceptions.json`. No test budget, enforcement code, workflow, product behavior or other exception changes. The [recorded approval](#12777 (comment)) authorizes exactly two live assertions for native host-local success and sibling isolation. Main #12608 subsequently removed seven unrelated assertions. This entry preserves that reduction and records the mechanically recalculated transition for signed Slice 4 candidate `3f967908b29892c28f57aa4f6457532f4811c6cb`: - Base budget SHA-256: `53c51c4229d387633165771730f8f1a8373f84e9f3303b2144280027a56e3441`. - Candidate budget SHA-256: `8bdce2cee059bb402f1f504e267f99cc39f1f7bf98928ffe0fb1bc95674dfa80`. - Only `inference-routing.test.ts` changes relative to current main: `[36,51,48,89,15]` to `[38,53,50,91,15]`. Direct assertion points increase from 1959 to 1961; unique points from 3203 to 3205. A different PR or budget digest cannot use this entry. Remove it after #12777 merges. ## Verification - `npx vitest run --project integration test/automation/pull-requests/growth-guardrails.test.ts test/automation/pull-requests/growth-guardrail-parsers.test.ts`: 65 tests pass. - Local pure-check rehearsal: current trusted policy reports eight expected growth violations; the proposed exact exception on the trusted side reports zero. No GitHub status was changed. - Compared both budget files and their complete structural difference. Only the two approved inference-routing assertions and corresponding aggregate counters increase. - Normal signed-commit hooks, including growth enforcement and secret scanning, pass. The diff contains no secrets, API keys or credentials. ## Review notes The required upstream APT setup fix from #12837 is integrated through main `c518e121d23c616da1b083c44c9eb504fb419ebd`. The exception conflict preserves every existing main entry and the unchanged #12777 digests. All 65 focused tests and normal signed-commit hooks pass on merge commit `b96598a2037a7e8347f8a8492624ff6c59ecb567`; latest-head CI remains required after publication. This is a workflow-authored transcription and updated disposition of Rebecca Sliter's existing approval, not a new independent review or a passing CI result. Current #12777 growth failures remain failures until trusted policy is available and enforcement passes. This prerequisite requires its own CI and automated review. Repository-local setup and compilation completed. The contributor doctor reports an unavailable local Docker daemon and a CLI resolving to another installation; neither is used by this policy-only validation. No runtime or CLI exposure was changed. --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Updated the automated test-baseline allowance for the approved inference-routing assertions. The allowance applies only when the recorded base and head digests match, and continues to account for the reduction in assertions from the prior baseline. Independent validation continues to use the trusted baseline policy. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## Outcome CPU DCode onboarding preserves the sandbox created by OpenShell, and native NVIDIA inference uses the supervisor-issued credential placeholder throughout configuration, model construction, and health checks. Rebuild preflight reuses the recorded native provider identity and rejects replacement or expired credentials before deleting the sandbox. ## Reason On Brev, forcing Docker recreation left an OpenShell 0.0.116 sandbox in terminal Error before chat could run. Subsequent live testing exposed native-provider policy, constructor authentication, response-format, and rebuild lookup errors. The managed DCode entrypoint already enforces the required process limits without recreating the CPU sandbox. ### Related issues Refs #11847, #6545. ## Changes - Preserve native CPU startup while retaining exact ulimits for the separately selected GPU compatibility recreation path. Composed creation tests reject accidental container discovery or replacement. - Enable native NVIDIA provider policy when unset, preserve an explicit administrator disable, and validate the same-name supervisor credential placeholder in generated configuration, the hardened model constructor, and both native probes. Shell-level regressions accept canonical and supported generation-scoped NVIDIA placeholders while rejecting foreign names, malformed scopes, suffixes, and newlines before use. - Carry the native provider receipt through rebuild and keyless onboarding; reject malformed persisted authority, then validate provider UUID, profile, credential metadata, and expiry again before deletion. Composed rebuild tests protect the existing sandbox before any backup, deletion, or recreation. Select the native registration only for a recorded native route; legacy managed `nvidia-prod` keeps its original registration. - Omit the Ultra response-template workaround for the native NVIDIA endpoint and allow 256 tokens for its readiness probes. Keep existing managed-route behavior. Executable constructor, config, and probe tests cover both routes. - Update native-provider test fixtures to implement the policy operation so existing transaction and failure-path assertions reach their intended behavior. ## Verification - Repair `061c21eb0`: 273 focused and related tests passed, plus 7 growth checks and 3 compiled-package checks. Normal publication validation and CLI typecheck passed without bypassing hooks or raising budgets. Earlier documentation validation passed with zero errors and two warnings. No secrets are included in the diff or retained evidence. - [Current-candidate CI passed](https://github.com/NVIDIA/NemoClaw/actions/runs/37832472363), including all 12 CLI shards. [Docker and rootless Podman activation passed](https://github.com/NVIDIA/NemoClaw/actions/runs/37832471647); those activation jobs use synthetic inference. - [All nine Advisor specialists passed with zero findings](https://github.com/NVIDIA/NemoClaw/actions/runs/37834928709). CodeRabbit confirmed all three repaired findings; the inline threads are resolved. Rebecca has not approved the repaired commit. - Real Brev validation on `061c21eb00bef25a5bef6201ca4ba7e20d293719`, OpenShell 0.0.116, October 8: hosted inference preflight returned HTTP 200, fresh onboarding passed, headless inference returned exact JSON `PONG`, all four process-limit paths passed, and QuickJS tool execution/REPL restoration passed while `memfd_create` remained blocked. - Canonical two-session interactive acceptance **failed: 10 predicates passed, 3 failed**. Session 1 completed its model response and exited cleanly; session 2 returned `APIError: An internal error occurred`. Process cleanup and secret checks passed. This does not establish reliable interactive chat. - Trusted default denial of tool auto-approval passed. Rebuild then received HTTP 503 from its inference probe and stopped before deletion. Actual tool opt-in, recreation, and baseline restoration were not reached. - Public OpenShell stop/start preserved sandbox UUID, durable marker, exact entrypoint limits, and blocked-memfd QuickJS execution. Its final inference invocation returned HTTP 503, so restart inference acceptance failed. - Public destroy removed the owned test sandbox and container. Temporary bundle and raw TUI captures are absent; the existing Brev VM and unrelated sandboxes remain. Shared gateway provider credentials were retained; the user must revoke the approved test key after the run. ## Review notes Candidate: NVIDIA/NemoClaw `061c21eb00bef25a5bef6201ca4ba7e20d293719`. CI and automated review are green, but reliable real interactive inference, tools/rebuild, and restart inference acceptance remain incomplete. The latest live evidence above is from this exact candidate. It is a manual development checkout on an existing Brev instance, not qualification of a newly baked Launchable image. The original blocked-memfd runtime checks pass; the remaining inference/API failures are not a complete live acceptance pass. Their underlying service/proxy cause has not been established. No unchanged live retry was performed. Merged #12837 is already integrated from canonical `5f765a1318a2338114a1f6865de3113d9083b405`; no further dependency PR is required for that installer fix. Self-review covers the sensitive `agents/langchain-deepagents-code/**`, `src/lib/onboard/**`, `src/lib/inference/**`, and provider-adapter changes, including credential ownership, native provider identity, policy composition, and startup limits. No independent approval is claimed. Rebecca’s [review of `b86d3dc`](#12793 (review)) confirmed three CodeRabbit findings; this candidate addresses all three with regression tests. All nine [Advisor specialists for `b86d3dc`](https://github.com/NVIDIA/NemoClaw/actions/runs/37820242524) completed successfully: three findings duplicate the credential-placeholder repair. Rebecca did not confirm the separate hard-limit finding because the exact-candidate live evidence passes all four limits. The repaired sensitive paths await independent re-review. The separately selected GPU compatibility recreation lifecycle remains outside this CPU Brev fix. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Native NVIDIA inference uses the current supervisor-managed credential and rejects missing or invalid credentials before use. * Native NVIDIA provider setup confirms gateway policy support before configuring the provider. * Sandbox rebuilds can reuse a recorded native NVIDIA provider when its identity and credentials remain valid. * **Bug Fixes** * NVIDIA Ultra health checks now use a model-appropriate response budget. * CPU Deep Agents startup limits are applied at launch without replacing the container; GPU compatibility limits remain supported. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Outcome
Restore the pinned Pi search-tool install on Ubuntu CLI shards by resolving the runner's
mirror+filearchive URI to the official Ubuntu archive in the isolated APT source file. The package pins, release suites, components, signing key, and security source remain in place.Reason
The first fix in #12778 merged, but main CI run 37782374203 stopped all 12 CLI shards before tests. APT refreshed package metadata, then package installation failed with exit 100 while downloading a
mirror+fileauxiliary mirror file from the isolated lists directory. This is an implementation defect in the #11320 infrastructure repair.Related issues
Fixes #11320. Follow-up to #12778.
Changes
.sourcesfile. The pinned CLI shard and Advisor installers need an APT source that can fetch packages while using isolated lists; changing the machine's source would affect unrelated jobs. Replace every occurrence of the expected URI, including the security stanza, and fail closed on a different mirrorlist URI._aptuser and remove it after the command. Preserve the existing bounded update/install supervision.mirror+fileinstall failure and assert that the rewritten source preserves suites, components, signing key, and security updates.Verification
npx vitest run --project integration test/automation/pull-requests/pr-workflow-contract.test.ts— 69 tests passed.bash -n .github/actions/ci-install-pinned-ubuntu-packages.sh,git diff --check, and Oxfmt check — passed.Review notes
Sensitive path:
.github/actions/ci-install-pinned-ubuntu-packages.sh. I inspected the exactd38857f753c7d779a117f78b0f097dac71d56efadiff inNVIDIA/NemoClawagainst canonicalmain; it confines the Ubuntu mirrorlist rewrite to a temporary source and adds a regression fixture. The first live manual Advisor run 37788544594 exposed a candidate defect: the runner uses the expected mirrorlist URI in multiple stanzas. This exact head repairs that case. Manual Advisor run 37790303426 checked outd38857f, executed the revised helper on Ubuntu, and completed pinned APT update and install. All nine Advisor specialists reported zero findings, and the blocker gate passed. CodeRabbit completed its exact-head review with no actionable code comments. PR shard jobs execute the base-pinned action; exact-head PR CI run 37790162279 therefore still fails all 12 shards before tests and its requiredchecksjob is red. Post-mergemainCI must verify that the revised helper reaches and completes CLI tests, especially shards 1, 5, 8, and 12.Signed-off-by: Rebecca Sliter 571084+rsliter@users.noreply.github.com
Summary by CodeRabbit