Skip to content

fix(inference): validate injected llama lifecycle authority - #9174

Merged
prekshivyas merged 1 commit into
mainfrom
fix/llama-lifecycle-review-followup
Aug 14, 2026
Merged

fix(inference): validate injected llama lifecycle authority#9174
prekshivyas merged 1 commit into
mainfrom
fix/llama-lifecycle-review-followup

Conversation

@ericksoa

@ericksoa ericksoa commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

This follow-up completes the outstanding actionable review feedback from #9123. Injected llama.cpp lifecycle operations are now revalidated against the sandbox-bound runtime provider before rehydration or startup, while unchanged legacy receipt dispatch remains intact.

Related Issue

Follow-up to #9123 and #7744.

Changes

  • Reuse the runtime-provider operation validator for injected operations at both llama.cpp rehydration and onboarding startup boundaries.
  • Reject same-provider operations whose engine operation, ID, or display name differs from the provider bundle, with focused rehydration and onboarding regression coverage.
  • Prove malformed durable receipts already fail before sandbox deletion, without changing legacy llama.cpp dispatch.
  • Bind lifecycle-adapter fixtures to one explicit gateway port and remove redundant Vitest mock teardown hooks.

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: Internal authority validation and test-fixture corrections do not change commands, configuration, defaults, output, or documented lifecycle selection.
  • 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: Follow-up validates the engine-authority findings from feat(runtime): preserve host-local inference lifecycle #9123 (comment) and feat(runtime): preserve host-local inference lifecycle #9123 (comment) at both production boundaries.
  • 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: Internal provider-authority validation and test-only corrections; no command, setting, default, output, installation step, or documented behavior changes.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

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: npm run build:cli && npm run typecheck:cli; focused Vitest projects passed 45 CLI tests and 52 integration tests; npm run checks:repository and npm run test-size:check passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • 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: Aaron Erickson aerickson@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • Prevented sandbox deletion and cleanup when host-local inference receipts are malformed.
    • Rejected host-local inference operations with mismatched engine authorities before startup or lifecycle changes.
    • Improved operation routing consistency during inference setup and rehydration.
    • Ensured lifecycle state and retry handling use the correct gateway-specific paths.
  • Tests

    • Added regression coverage for invalid receipts, authority mismatches, routing, startup, rollback, and destroy-retry scenarios.

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa ericksoa self-assigned this Aug 14, 2026
@coderabbitai

coderabbitai Bot commented Aug 14, 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: 6de376a2-0a46-430f-a4c7-2436588886a0

📥 Commits

Reviewing files that changed from the base of the PR and between 3df4b3f and 836ef45.

📒 Files selected for processing (8)
  • src/lib/actions/sandbox/destroy-host-local-inference.test.ts
  • src/lib/actions/sandbox/snapshot-command-host-local-authority.test.ts
  • src/lib/inference/llama-cpp/managed-installer.test.ts
  • src/lib/inference/llama-cpp/managed-installer.ts
  • src/lib/inference/llama-cpp/managed-lifecycle-adapter.test.ts
  • src/lib/onboard/runtime-provider/registry.ts
  • src/lib/onboard/setup-inference.ts
  • test/onboard-host-local-inference-routing.test.ts

📝 Walkthrough

Walkthrough

The change preserves supplied host-local inference operations during setup and rehydration. Regression tests cover mismatched engine identities, malformed receipts, gateway-port-specific state paths, and updated test harness setup.

Changes

Host-local inference validation

Layer / File(s) Summary
Preserve supplied inference operations
src/lib/onboard/runtime-provider/registry.ts, src/lib/onboard/setup-inference.ts, src/lib/inference/llama-cpp/managed-installer.ts
Runtime-provider resolution accepts an optional operation candidate. Setup and rehydration pass the adapter operation for authority validation.
Reject mismatched engine authorities
test/onboard-host-local-inference-routing.test.ts, src/lib/inference/llama-cpp/managed-installer.test.ts
Tests verify that mismatched engine identities fail before startup, gateway mutation, command execution, or lifecycle construction.
Align cleanup and state-path test coverage
src/lib/actions/sandbox/destroy-host-local-inference.test.ts, src/lib/inference/llama-cpp/managed-lifecycle-adapter.test.ts, src/lib/actions/sandbox/snapshot-command-host-local-authority.test.ts
Tests cover malformed receipt handling, gateway-port-specific state paths, and simplified mock setup.

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

Merge Risk: ⚪ Minimal · up to 836ef

The change revalidates injected llama lifecycle authority before rehydration and startup, with focused coverage reported as passing; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: apurvvkumaria, brandonpelfrey, cv, dnandakumar-nv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: validating injected llama.cpp lifecycle authority.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/llama-lifecycle-review-followup

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

@github-code-quality

github-code-quality Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 836ef45 in the fix/llama-lifecycle-... branch remains at 96%, unchanged from commit 711bc75 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 836ef45 in the fix/llama-lifecycle-... branch remains at 82%, unchanged from commit 93461f3 in the main branch.

Show a code coverage summary of the most impacted files.
File main 93461f3 fix/llama-lifecycle-... 836ef45 +/-
src/lib/inferen...hugging-face.ts 98% 92% -6%
src/lib/actions...box/snapshot.ts 83% 82% -1%
src/lib/cua/run...ime-manifest.ts 84% 90% +6%
src/lib/inferen...file/cleanup.ts 78% 85% +7%
src/lib/cua/bounded-file.ts 84% 94% +10%
src/lib/actions...oy-execution.ts 71% 83% +12%
src/lib/inferen...ycle-adapter.ts 0% 77% +77%
src/lib/state/r...al-inference.ts 0% 92% +92%
src/lib/onboard...ce-lifecycle.ts 0% 93% +93%
src/lib/gateway...art-guidance.ts 0% 95% +95%

Updated August 14, 2026 19:33 UTC

@github-actions

github-actions Bot commented Aug 14, 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 · 1 warning · 1 suggestion
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections match; Nemotron reported the same number of blockers, 1 more warning, 1 more suggestion.
1 terminology difference from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • engine drift at test/onboard-host-local-inference-routing.test.ts:650: selected only by the second-opinion lane as justified.

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

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • established — host-local-inference authority at src/lib/inference/llama-cpp/managed-installer.test.ts:787: Keep this established term for authority validation that rejects mismatched injected operations.
  • established — host-local-inference authority at test/onboard-host-local-inference-routing.test.ts:680: Keep this established term for authority validation that rejects mismatched injected operations.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: managed-image-protected-runtime, inference-routing

Manual-only E2E: managed-image-multiarch-startup, onboard-repair, onboard-resume, cloud-onboard, network-policy
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

3 optional E2E recommendations
  • llama-cpp-dgx-spark-qualification
  • llama-cpp-generic-gpu
  • snapshot-commands

Workflow run details

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

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 836ef45. No blocking findings; required checks are green.

@prekshivyas
prekshivyas merged commit af31708 into main Aug 14, 2026
81 checks passed
@prekshivyas
prekshivyas deleted the fix/llama-lifecycle-review-followup branch August 14, 2026 20:36
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants