Skip to content

test(onboard): preserve runtime fixture isolation - #8557

Merged
jyaunches merged 1 commit into
mainfrom
codex/fix-onboard-runtime-clone
Aug 7, 2026
Merged

test(onboard): preserve runtime fixture isolation#8557
jyaunches merged 1 commit into
mainfrom
codex/fix-onboard-runtime-clone

Conversation

@jyaunches

@jyaunches jyaunches commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

cloneSession from #8546 returned the caller-owned session when normalization rejected its serialized copy. This follow-up keeps the serialized copy as the fallback so fixture mutations cannot alter caller-owned top-level or nested state.

Changes

  • Use the serialized session copy as both the normalization input and fallback.
  • Add a regression test with an invalid session version. The test proves that normalization rejection still returns independent top-level and nested state.
  • Record the detection gap: test(onboard): add an in-memory machine runtime fixture #8546 exercised valid sessions only, so normalization always succeeded and did not reach the aliasing fallback.
  • Close the complete test(onboard): add an in-memory machine runtime fixture #8546 review set. The CodeRabbit isolation finding is fixed. The docstring-coverage warning is advisory because local exports have JSDoc and re-export documentation remains source-owned. The PR Review Advisor reported no findings and failed only because its model request returned HTTP 429.
  • Inspect the equivalent local clone helpers in test/helpers/onboard-final-flow-phases.ts, src/lib/onboard/resume-machine-repair.test.ts, src/lib/onboard/runtime-boundary-step-result.test.ts, and src/lib/onboard/runtime-boundary.test.ts. Their callers construct valid sessions and do not expose test(onboard): add an in-memory machine runtime fixture #8546's independent-mutation contract, so this PR does not change them.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • 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:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: this changes an internal test fixture and its regression coverage. No CLI, configuration, output, workflow, default, API, policy schema, or supported user behavior changes.
  • 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:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: the completed diff changes only test/helpers/onboard-machine-runtime-fixture.ts and test/helpers/onboard-machine-runtime-fixture.test.ts. The reviewer found no user-visible behavior or documentation impact and approved the test title and existing JSDoc ownership.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; scripts/prepare-dgx-station-host.sh does not change.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • 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
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project integration test/helpers/onboard-machine-runtime-fixture.test.ts passed 1/1; the eight test(onboard): add an in-memory machine runtime fixture #8546 CLI suites passed 63/63; npm run typecheck:cli passed; npm run test:projects:check reported exact membership for 2,231 candidate files across seven projects.
  • 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. The change updates one fixture fallback and adds one focused test; targeted validation covers every test(onboard): add an in-memory machine runtime fixture #8546 consumer.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Improved session cloning so copied sessions remain independent even when normalization fails.
    • Prevented changes to a cloned session from affecting the original session.
  • Tests

    • Added regression coverage for cloning invalid-version sessions and nested steps.

Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
@jyaunches jyaunches self-assigned this Aug 7, 2026
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c542674b-8ed9-4e1f-a483-35e7cc2cc2e1

📥 Commits

Reviewing files that changed from the base of the PR and between 0da798a and 64c19cf.

📒 Files selected for processing (2)
  • test/helpers/onboard-machine-runtime-fixture.test.ts
  • test/helpers/onboard-machine-runtime-fixture.ts

📝 Walkthrough

Walkthrough

cloneSession now preserves its deep copy when normalization fails. A regression test verifies that mutating the clone does not change the original session or its nested steps.

Changes

Clone session isolation

Layer / File(s) Summary
Clone fallback and regression coverage
test/helpers/onboard-machine-runtime-fixture.ts, test/helpers/onboard-machine-runtime-fixture.test.ts
cloneSession returns the deep-copied session when normalization returns no value. The regression test verifies isolation of the session and nested steps after mutation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#8546: Directly modifies and tests the shared cloneSession helper addressed by this change.

Suggested reviewers: aasthajh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the onboard runtime fixture isolation fix and matches the main change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-onboard-runtime-clone

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 64c19cf in the codex/fix-onboard-ru... branch remains at 96%, unchanged from commit 0da798a in the main branch.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized terminology decisions match; normalized E2E selections match; severity counts match.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: None

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@jyaunches
jyaunches merged commit 61ae0ee into main Aug 7, 2026
64 of 67 checks passed
@jyaunches
jyaunches deleted the codex/fix-onboard-runtime-clone branch August 7, 2026 13:46
@github-actions github-actions Bot added the v0.0.105 Release target label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v0.0.105 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant