fix(e2e): repair historical gateway upgrades - #7362
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:
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 (1)
📝 WalkthroughWalkthroughThe legacy installer fixture adapter now validates exact reviewed NemoClaw/OpenClaw profiles, stages historical archives under the frozen build context, applies profile-specific advisory-audit handling, and verifies successful and rejected fixture executions. ChangesLegacy installer fixture updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UpgradeTest
participant FixtureValidator
participant FixturePatcher
participant BuildContext
participant InstallerDockerfile
UpgradeTest->>FixtureValidator: validate reviewed fixture identity
FixtureValidator->>FixturePatcher: pass exact profile
FixturePatcher->>BuildContext: stage historical archive
FixturePatcher->>InstallerDockerfile: inject archive COPY and audit handling
InstallerDockerfile->>BuildContext: install reviewed OpenClaw archive
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7362.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 2679a4b in the TypeScript / code-coverage/cliThe overall coverage in commit 2679a4b in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts`:
- Around line 125-135: Replace the manual fs.mkdirSync/fs.cpSync staging in the
test with the real frozen optimized-context builder used by the upgrade flow,
then assert that its staged output contains OLD_INSTALLER_ARCHIVE_CONTEXT_PATH
with the expected content. Keep the test’s existing fixture and archive
assertions intact while exercising the builder boundary rather than recreating
its copy behavior.
🪄 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: 4a9f6fa4-ad8a-4260-8983-e313ae721743
📒 Files selected for processing (4)
docs/security/openclaw-2026.6.10-dependency-review.mdtest/e2e/live/openshell-gateway-upgrade-old-installer.tstest/e2e/live/openshell-gateway-upgrade.test.tstest/e2e/support/openshell-gateway-upgrade-old-installer.test.ts
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
One blocking release-safety issue remains on this security-sensitive fixture boundary:
patchOldInstallerFixture() selects the allowed advisory-audit shape from NEMOCLAW_OLD_NEMOCLAW_REF, but the installer and Dockerfile being patched are selected independently by NEMOCLAW_OLD_NEMOCLAW_COMMIT. validateLegacyGatewayUpgradeFixture() currently validates only field formats, not the exact reviewed ref/commit/OpenClaw tuple. A mixed tuple can therefore apply the 0-audit policy for one release to another release's Dockerfile, weakening the fail-closed review boundary and allowing the regression lane to validate the wrong historical payload.
Please bind the policy to the full pinned commit SHA, or require an exact reviewed { ref, commit, openclawVersion } profile and pass that profile through to the adapter. The support success cases should use the real reviewed tuple for each release rather than pairing v0.0.36/v0.0.55 with OpenClaw 2026.5.27.
I reviewed the remaining implementation against all three frozen stagers. The nemoclaw/src archive path survives v0.0.36, v0.0.55, and v0.0.74, the 0/0/1 audit counts match the pinned Dockerfiles, and v0.0.74 retains signature verification. The manual staging copy noted by CodeRabbit is worth replacing with the real/frozen stager contract, but it is fix-forward rather than blocking for these three immutable profiles because direct inspection confirms their copy behavior.
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
Addressed the requested release-safety binding in 9b5f7db. The fixture validator and installer adapter now both require an exact reviewed |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts (1)
120-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStaged path relies on raw stdout as the only IPC channel.
stageFrozenOptimizedBuildContextcapturesresult.stdoutverbatim as the staged context path. Any incidentalconsole.log/stdout write inside the dynamically-imported historical module (now or in the future) would corrupt this value and breakfs.readFileSync(path.join(stagedContext, ...))downstream. Consider writing the result to a temp file (or taking only the last line) instead of trusting raw stdout.♻️ Proposed fix: relay the staged path via a file instead of stdout
function stageFrozenOptimizedBuildContext( sourceRoot: string, nemoclawRef: ReviewedHistoricalRef, ): string { const modulePath = path.join(sourceRoot, HISTORICAL_BUILD_CONTEXT_MODULES[nemoclawRef]); + const outputFile = path.join(os.tmpdir(), `nemoclaw-staged-context-${crypto.randomUUID()}.txt`); const runner = String.raw` import { pathToFileURL } from "node:url"; +import { writeFileSync } from "node:fs"; const modulePath = process.argv[1]; const sourceRoot = process.argv[2]; const temporaryRoot = process.argv[3]; +const outputFile = process.argv[4]; const buildContext = await import(pathToFileURL(modulePath).href); const staged = buildContext.stageOptimizedSandboxBuildContext(sourceRoot, temporaryRoot); -process.stdout.write(staged.buildCtx); +writeFileSync(outputFile, staged.buildCtx); `; const result = spawnSync( process.execPath, [ "--no-warnings", "--experimental-strip-types", "--input-type=module", "--eval", runner, modulePath, sourceRoot, path.dirname(sourceRoot), + outputFile, ], { encoding: "utf8" }, ); expect(result.status, result.stderr).toBe(0); - return result.stdout; + return fs.readFileSync(outputFile, "utf8"); }🤖 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/e2e/support/openshell-gateway-upgrade-old-installer.test.ts` around lines 120 - 151, Update stageFrozenOptimizedBuildContext to relay the staged build-context path through a temporary file rather than treating result.stdout as the sole IPC channel. Pass the file location to the spawned runner, have the runner write staged.buildCtx there, then read and return that file’s contents while preserving the existing nonzero-status assertion and cleanup.
🤖 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 `@test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts`:
- Around line 102-118: Update the OpenShell gateway upgrade job checkout in the
e2e workflow to fetch tags, using fetch-depth: 0 or fetch-tags: true, so
extractReviewedHistoricalSource can resolve historical refs such as v0.0.36,
v0.0.55, and v0.0.74 in shallow checkouts.
---
Nitpick comments:
In `@test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts`:
- Around line 120-151: Update stageFrozenOptimizedBuildContext to relay the
staged build-context path through a temporary file rather than treating
result.stdout as the sole IPC channel. Pass the file location to the spawned
runner, have the runner write staged.buildCtx there, then read and return that
file’s contents while preserving the existing nonzero-status assertion and
cleanup.
🪄 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: 9f72bc97-e820-4278-bd5b-c18eaca91f31
📒 Files selected for processing (1)
test/e2e/support/openshell-gateway-upgrade-old-installer.test.ts
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `v0.0.91` changelog entry that was missed before the release tag was cut. Correct the custom-image compatibility guidance because the tagged code still accepts the legacy inference route selector instead of removing it in v0.0.91. ## Changes - Add `docs/changelog/2026-07-22.mdx` with the release summary and detailed security, rebuild, Hermes, DGX Station, and historical-validation changes from the published v0.0.91 announcement. - Link shipped behavior to the most specific published OpenClaw and Hermes documentation routes. - Correct the v0.0.90 changelog and command reference so they match the compatibility fallback present in the tagged v0.0.91 code without inventing a new removal version. - Keep the immutable v0.0.91 release tag unchanged; this is the documented post-release recovery path. Source summary: - [#7332](#7332), [#7289](#7289), and [#7294](#7294) -> `docs/changelog/2026-07-22.mdx`: Summarize completed-image `node-tar` remediation and current and historical container verification. - [#7213](#7213), [#7363](#7363), [#7366](#7366), and [#7369](#7369) -> `docs/changelog/2026-07-22.mdx`: Summarize trusted base preparation, backup reuse, deletion convergence, and rebuild confidence. - [#7212](#7212) -> `docs/changelog/2026-07-22.mdx`: Summarize the Hermes API bearer-token lifecycle and supported retrieval command. - [#7327](#7327) and [#7328](#7328) -> `docs/changelog/2026-07-22.mdx`: Summarize qualified DGX Station guidance and reproducible coding-agent installation instructions. - [#7355](#7355), [#7360](#7360), [#7362](#7362), and [#7364](#7364) -> `docs/changelog/2026-07-22.mdx`: Summarize restored historical OpenClaw upgrade and Hermes rebuild validation. - [#7189](#7189) -> `docs/changelog/2026-07-20.mdx`, `docs/reference/commands.mdx`: Correct its forward-looking removal deadline after v0.0.91 shipped with the documented legacy fallback still present. - [#7282](#7282), [#7306](#7306), and [#7341](#7341) need no additional user-guide update because they already update their owned contributor or user-facing text directly. ## 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 - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, SPDX syntax, version ordering, and published routes. - [ ] Tests not applicable — justification: - [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: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this documentation-only PR does not change Station preparation or runtime behavior. - 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 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 — `npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not run; this is a focused 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) — completed with 0 errors and 2 existing site-wide 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) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated v0.0.91 guidance for custom Dockerfiles, including continued legacy compatibility and recommended migration to `NEMOCLAW_INFERENCE_PROVIDER_ID`. * Added release notes covering image security scanning, safer rebuild behavior, token management, DGX Station guidance, and deterministic release validation. * Clarified that existing custom images may continue using the legacy selector temporarily, with fallback removal planned for a future release. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
wscurran
added
area: ci
3 participants
Summary
Repair the historical gateway-upgrade adapter after E2E main run 29890967158 exposed four deterministic failures.
The adapter now matches each pinned release's advisory-audit shape and stores the reviewed OpenClaw archive in the source subtree preserved by frozen optimized build contexts.
Related Issue
Follow-up to #7360.
Changes
v0.0.36andv0.0.55require zero advisory-audit statements, whilev0.0.74requires one.A direct one-count rule cannot support the older frozen Dockerfiles because they predate the mcporter audit block.
The E2E-support test covers every reviewed profile and rejects unknown profiles or mismatched counts.
nemoclaw/srcand update the injected DockerfileCOPYpath.Each frozen optimized-context builder copies that subtree, while none copies arbitrary source-root files.
The E2E-support test stages the preserved subtree and verifies that the archive remains available.
v0.0.74signature-verification scope.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 check:diffpassed when hooks were skipped or unavailablenpx vitest run --project e2e-support test/e2e/support/openshell-gateway-upgrade-old-installer.test.tspassed 11 tests.e2e-phase-lifecyclemissing-log failure after serial confirmation. The four other parallel timeout failures passed serially.npm run docsbuilds without warnings (doc changes only) — the build passed with two existing Fern warnings.Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit