fix(onboard): sync Homebrew formula pin - #9882
Merged
Merged
Conversation
Keep lifecycle trust aligned with the selected OpenShell release. Reject future formula-pin drift and trust the exact repair template for the next stack layer. Refs #9848 Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
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 (6)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughOpenShell dependency validation now compares the trusted release formula checksum with the gateway service pin. The v0.0.106 installer trust record accepts an additional template digest. Tests cover stale formula pins and trusted formula reuse. ChangesOpenShell trust validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change updates the Homebrew formula pin and its trust validation with targeted tests and repository checks reported as passing; no actionable merge-blocking risk remains beyond normal review. Sequence Diagram(s)sequenceDiagram
participant DependencyChecks
participant OpenShellValidation
participant ReleaseTrust
participant GatewayService
DependencyChecks->>GatewayService: Read source
DependencyChecks->>OpenShellValidation: Pass gateway service source
OpenShellValidation->>ReleaseTrust: Extract trusted formula checksum
OpenShellValidation->>GatewayService: Read pinned formula checksum
OpenShellValidation-->>DependencyChecks: Return validation result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit db84240 in the TypeScript / code-coverage/cliThe overall line coverage in commit db84240 in the Show a line coverage summary of the most impacted files.
Updated |
laitingsheng
added
area: install
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
2 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Collaborator
Maintainer Dependency, Security, and Documentation ReviewVerdictPASS for the current branch revision. I found no technical, security, or documentation change request. The pull request is not merge-ready until fresh required checks pass and repository-routed independent review covers the current revision. Dependency Evidence
Security Review
Documentation ReviewNo documentation change is needed. This pull request aligns an internal artifact-integrity value and adds repository enforcement for existing supported behavior. It does not change a public command, operator procedure, or supported product surface. Validation
E2E ClassificationThe earlier OpenClaw trusted-private MCP discovery jobs failed twice in the concurrent-add phase. Both operations were rejected because OpenShell did not synchronize the expected credential revision. The pull request does not change MCP, credential synchronization, Linux runtime selection, or the managed image used by those jobs. Cleanup passed, and both artifact secret scans passed. This matches open issue #9764. The repeated result is an external technical blocker, not a confirmed transient failure, so I did not rerun it. Fresh checks are running after the branch update. |
jyaunches
approved these changes
Aug 21, 2026
20 tasks
jyaunches
added a commit
that referenced
this pull request
Aug 22, 2026
<!-- markdownlint-disable MD041 --> ## Summary The rebuild handoff could restart a healthy replacement container while OpenShell still processed the rollback backup deletion. NemoClaw now requires a successful OpenShell sandbox list that omits the selected sandbox name before it restarts the replacement. During onboarding, lifecycle polling and final readiness use the same handoff deadline. Legacy recovery also requires lifecycle release and final readiness before success. ## Related Issue Related to #9531. ## Changes - Add a bounded, fail-closed lifecycle-release check that accepts only an explicit empty sandbox list or a parsed list that omits the exact sandbox name. - Require affirmative lifecycle-release evidence before either onboarding or legacy recovery restarts the replacement; missing or failed evidence remains fail-closed. - Preserve the existing replacement stop and rollback backup removal before the release check. After release evidence is established, preserve replacement restart and final supervisor readiness as separate authorities. - Share the existing onboarding final-handoff deadline between lifecycle release and final readiness, and keep polling sleep on the host after the previous container is retired. - Add behavior tests for every accepted lifecycle-release receipt, neighboring malformed and phase-free outputs, failed and missing probe statuses, `Deleting` to `Error` to name-absence ordering, exact injected runner identity, and composed-flow success suppression. - Update eight embedded messaging create/recreate runner fixtures to return the canonical successful empty sandbox-list receipt required by lifecycle release. - Record the workaround contract, its regression tests, and the upstream condition that permits removal. - Document the final lifecycle-release step in the existing command reference. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent nine-category security review passed for commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493` against current base `465d7112f321d9946c5b130d87ce543de3adf38e`. The follow-up changes only embedded test fixtures and adds no production authority. The PR net adds no credential, authorization, dependency, cryptography, configuration, or privilege boundary. Both onboarding and legacy recovery require the same successful name-absence receipt before restart, failed probes remain fail-closed, polling uses a host-bound sleep, and final readiness is still required before success publication. Onboarding bounds lifecycle polling and final readiness with one handoff deadline. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; no `scripts/prepare-dgx-station-host.sh` change. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: - Commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493` is a signed/DCO follow-up to reconciliation `053d704d46ba2d0ea4b7159a80b6cbaa802d7ff3`, whose ordered parents are [`aa533f9923bec620b49a9cebe51b81297e533910`, `465d7112f321d9946c5b130d87ce543de3adf38e`]. The original 11 lifecycle, recovery, and documentation blobs are byte-identical to the reviewed first-parent candidate. The exact net against current base is 12 files, +489/-43; the twelfth file is the fixture-only eight-for-eight replacement in `test/onboard-messaging.test.ts`. - On exact reconciliation `053d704d46ba2d0ea4b7159a80b6cbaa802d7ff3`, the focused #9770 messaging recreate case reached the new fail-closed boundary but could not proceed because its embedded runner returned a blank successful sandbox list. The same focused case passed 1/1 on exact current main `465d7112f321d9946c5b130d87ce543de3adf38e`, which established that this was candidate-composition fixture debt. After the fixture correction, `npm exec -- vitest run --project integration test/onboard-messaging.test.ts -t "publishes attached OpenShell provider state before a messaging recreate starts"` passed 1/1, and `npm exec -- vitest run --project integration test/onboard-messaging.test.ts` passed 15/15 on commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493`. - The exact lifecycle helper/finalizer suites passed 43/43 and the current-main created-sandbox finalization suite passed 14/14 on commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493`. - Fail-first commit `e9f35928c9fb0f6058525d9190247f9f31ea92d5` added the ordering regression. The existing implementation passed 15 tests and failed the new test because it restarted the replacement without a lifecycle-release receipt. - `npm exec -- vitest run --project cli` with the 17 Docker GPU patch, lifecycle, and supervisor test files passed 202/202 on behavior commit `66436ff58990cc47fdbf44aa876b1629b9b4d692`. - Managed-image run `32460178030` at prior commit `7717276800f12e779b943151ed26fb689595c2ae` exposed the next state in the same lifecycle: both OpenClaw discovery passes reached a running replacement but OpenShell reported the exact sandbox in `Error`. The revised test failed 1/17 because the waiter did not accept that controlled stopped-replacement phase; commit `61af3e0a668ae66183b3ecd2b9c6a55d0e918c69` passed 18/18 after the correction and negative coverage for `Failed`. - The exact-`cd5ba235` implementation passed 41 prior tests and failed both new lifecycle assertions: it accepted the selected sandbox's `Error` row and restarted before the successful name-absence receipt. The initial corrected commit then failed the composed recovery diagnostic test because it reported a Docker start failure when restart was not attempted. First parent `1f7958ba87b6852d9b6dd910221e15d3699e5fe6` passed the corrected focused helper and finalizer suite, 43/43, plus the eight-case no-forward recovery table; those exact tested blobs are unchanged in the commit under review. - `npm exec -- vitest run --project cli src/lib/onboard/docker-gpu-*.test.ts src/lib/actions/sandbox/supervisor-relaunch.test.ts` passed 354/354 on first parent `1f7958ba87b6852d9b6dd910221e15d3699e5fe6`; those exact tested blobs are unchanged in the commit under review. - `npm exec -- vitest run --project integration test/process-recovery-supervisor-relaunch.test.ts test/brev-launchable-e2e.test.ts test/vitest-watch-triggers.test.ts` passed 132/132 on first parent `1f7958ba87b6852d9b6dd910221e15d3699e5fe6`; 29 tests cover recovery and 103 cover exact-base Launchable diagnostics composition, and those tested blobs are unchanged in the commit under review. - Composition with merged #9792 passed 19/19 isolated rebuild-lifecycle tests, 81/81 policy/create-intent tests, and 45/45 MCP E2E-support tests on reconciliation commit `5b1af40cf76446073be5a9d8b8ff8eb9f92908b8`. - Exact-candidate [managed-image run `32501429492`](https://github.com/NVIDIA/NemoClaw/actions/runs/32501429492) passed at `4d45294b47342cab74ed862296928065363803a5`, including all-agent activation and both OpenClaw trusted-private MCP discovery passes after #9792 merged. - Focused [manual PR E2E run `32504364196`](https://github.com/NVIDIA/NemoClaw/actions/runs/32504364196), attempt 1, passed the exact [`rebuild-openclaw` job `96841526032`](https://github.com/NVIDIA/NemoClaw/actions/runs/32504364196/job/96841526032). Immutable receipt `e2e-dispatch-32504364196-1` binds PR #9877, candidate `4d45294b47342cab74ed862296928065363803a5`, base and trusted workflow `c6dbeae8fc44ef8b0fca9813571bc4240c3a682a`, and selector `jobs=rebuild-openclaw`; unrelated targets and staging, Jetson, and DGX selectors were disabled. - Composition against base `f7ed928a8d94b9854ce243b7d94928a4969883c1` on prior PR commit `cd5ba23570a623d538b1ca0065e3278f7f592fe2` passed 211/211 inference, Hermes Portable, and Podman probe tests plus 38/38 Portable E2E-support tests. The exact-main `post-merge-docs` safe-integer case exceeded its fixed 15-second limit both in the 61-test composition run and alone on this macOS host; that test and its implementation are inherited unchanged from the base and do not enter the lifecycle path. - `npm exec -- vitest run --project cli src/lib/onboard/docker-driver-gateway-service-homebrew.test.ts` passed 9/9 on prior exact reconciliation `1f7958ba87b6852d9b6dd910221e15d3699e5fe6`. `npm exec -- vitest run --project integration test/dependency-pins-check.test.ts test/installer-homebrew-formula-reuse-trust.test.ts` passed 12/12. These tests cover #9882's separate Homebrew formula pin and trust checks. - `npm exec -- vitest run --project integration test/installer-hash-check.test.ts` passed 84/84 on retained reconciliation `aa533f9923bec620b49a9cebe51b81297e533910`, and host-bound `npm run check:installer-hash` accepted every pinned OpenShell v0.0.106 release asset. These tests cover #9777's separate installer-template trust digest. - `npm run validate:pr` passed for commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493`, including the CLI typecheck, source-shape check, repository checks, 32-test growth guardrail, and `git diff --check`. `npm run docs` passed on retained first parent `1f7958ba87b6852d9b6dd910221e15d3699e5fe6`; Fern reported zero errors and two warnings, and the candidate documentation blob is unchanged in the commit under review. Earlier behavior commit `66436ff58990cc47fdbf44aa876b1629b9b4d692` also passed `npm run build:cli` and `npm run docs`. - Commit `9565fd2ace214ef16b428478925e8416f31787f2` addresses the affirmative-release and shared-deadline findings. Commit `5e700a8ec0dda79f1c29302cd9f1e1b4f33e8fbb` addresses the exact-9565 host-bound sleep finding and its PR Review Advisor finding PRA-1. Commit `5b1af40cf76446073be5a9d8b8ff8eb9f92908b8` addresses the two exact-5e CodeRabbit test-evidence findings. Commit `7254137464264c114b3928e7c164d98ffe295de1`, preserved byte-for-byte in the commit under review, addresses exact-`cd5ba235` Advisor finding PRA-1 by rejecting every row that still carries the selected sandbox name and reporting that denial without claiming Docker start failed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable. This change updates one onboarding lifecycle boundary and its focused tests; it does not change a broad runtime or test harness. - [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) — `npm run docs` passed with zero errors; Fern reported two warnings. - [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) ### Source regression - Automatic Main run: https://github.com/NVIDIA/NemoClaw/actions/runs/32450387278 - Failed target job: https://github.com/NVIDIA/NemoClaw/actions/runs/32450387278/job/96679136735 - Tested main commit: `b29e37613301c26cdb401b93feecc9192e430355` - The replacement container was running and healthy, but OpenShell kept `e2e-rebuild-oc` in `Deleting` until the 18-minute final-handoff wait failed. Cleanup passed. - Automatic Main run https://github.com/NVIDIA/NemoClaw/actions/runs/32460591192 at exact base commit `fac4e6d6783e8909aabf4a9d94f5fa809fea3ec1` reproduced the same onboarding finalizer before Discord pairing: job https://github.com/NVIDIA/NemoClaw/actions/runs/32460591192/job/96713099795 reported a healthy replacement with the exact sandbox still in `Deleting`, then the same final-handoff failure. - Exact-candidate managed-image run https://github.com/NVIDIA/NemoClaw/actions/runs/32460178030 at prior commit `7717276800f12e779b943151ed26fb689595c2ae` reached the controlled stopped-replacement state in both OpenClaw discovery passes: OpenShell reported the exact sandbox in `Error` while the replacement container remained running. That evidence established the bounded lifecycle wait, but the exact-`cd5ba235` Advisor review found that a name-and-phase row cannot identify which container owns that lifecycle. Commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493` now requires the selected name to be absent. #9792 merged as `ebc600164bc08f4fb62c8078b2e0139a582b5486`; exact local rebuild, policy/create-intent, and MCP-support composition is green. The exact `rebuild-openclaw` target passed on reviewed commit `4d45294b47342cab74ed862296928065363803a5` in focused run `32504364196`. Commit under review `0e1cf95d699bd81fd29e747c9f51d9536fd11493` retains the lifecycle-release correction and adds current-main fixture compatibility, so exact-target E2E remains pending for that commit. --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved GPU sandbox handoff reliability by confirming release of the previous lifecycle record before restarting its replacement. * Added bounded lifecycle checks that handle command failures, incomplete results, and unrelated lifecycle states. * Prevented replacement restarts and final handoff when lifecycle release cannot be confirmed. * Preserved the overall handoff deadline during lifecycle and supervisor reconnection checks. * **Documentation** * Clarified GPU compatibility-recreation behavior during sandbox replacement. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
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
3 participants
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
NemoClaw previously passed the OpenShell v0.0.101 formula digest to the macOS gateway lifecycle after the runtime moved to v0.0.106, so onboarding rejected the valid staged formula. This layer supplies the v0.0.106 digest, prevents future consumer drift, and establishes the reviewed trust prerequisite for the next stack layer.
Changes
test/installer-homebrew-formula-reuse-trust.test.tsprotects this prerequisite.Type of Change
Quality Gates
DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm run check:installer-hashandnpm run checks:repositorypassed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests