feat(inference): add protected llama.cpp Spark qualification - #8266
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added
area: inference
📝 WalkthroughWalkthroughThe PR adds a protected llama.cpp DGX Spark qualification flow. It validates recipe data, compiles a digest-bound plan, runs trusted ARM64 qualification, verifies evidence and cleanup, and integrates workflow and risk-plan checks. ChangesDGX Spark qualification
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Controller
participant PlanJob
participant CandidateRepository
participant QualificationJob
participant DGXSpark
participant Evidence
Controller->>PlanJob: dispatch protected plan compilation
PlanJob->>CandidateRepository: checkout exact candidate configuration
PlanJob->>QualificationJob: publish digest-bound plan
QualificationJob->>DGXSpark: build and run pinned ARM64 candidate
DGXSpark->>Evidence: produce qualification receipt
QualificationJob->>Evidence: validate and upload artifacts
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit b470a75 in the TypeScript / code-coverage/cliThe overall coverage in commit b470a75 in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 4 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
scripts/checks/run-llama-cpp-dgx-spark-qualification.mts (2)
1067-1074: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider preserving a bounded failure reason.
The top-level catch discards the error and writes one generic line. Every failure mode in this 1000-line runner then produces the same operator-visible message. All thrown messages in this file are static strings that contain no secrets or paths. Writing
error.messageforErrorinstances keeps the lane debuggable without leaking run data.🤖 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 `@scripts/checks/run-llama-cpp-dgx-spark-qualification.mts` around lines 1067 - 1074, Update the top-level catch around main() to retain the caught error and append its message when it is an Error instance, while preserving the generic failure text for other thrown values. Keep the existing stderr output and exitCode behavior unchanged.
205-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the tautological registry-owner comparison.
Line 205 assigns
registryOwnerfromexpectedRegistryOwner(runId, runAttempt). Line 212 then compares the same value against the same call. That clause can never be false, so it does not verify ownership. Only theregistryNameclause has effect. Keep the check readable so a future reader does not treat the owner as independently validated.♻️ Proposed simplification
- if ( - registryOwner !== expectedRegistryOwner(runId, runAttempt) || - registryName !== expectedRegistryName(runId, runAttempt) - ) { + if (registryName !== expectedRegistryName(runId, runAttempt)) { throw new Error("registry ownership does not match this workflow run"); }🤖 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 `@scripts/checks/run-llama-cpp-dgx-spark-qualification.mts` around lines 205 - 216, Remove the tautological registryOwner comparison from the validation condition in the workflow check, since it compares expectedRegistryOwner(runId, runAttempt) with itself. Keep the registryName validation and simplify the surrounding logic so it clearly validates only the effective registry-name constraint.test/llama-cpp-dgx-spark-qualification-runner.test.ts (1)
263-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the vacuous secret-leak assertion.
Line 264 asserts that the argv does not contain the literal
raw-secret-value. No input tobuildServerContainerArgvever contains that literal, so this assertion cannot fail. It does not prove that the API key stays off argv. Pass a recognizable key material value through a real input and assert its absence, or drop the line and rely on the--api-key-fileassertion at line 262.♻️ Proposed change
expect(valuesAfter(argv, "--api-key-file")).toEqual(["/run/secrets/llama-cpp-api-key"]); expect(argv).not.toContain("--api-key"); - expect(argv.join(" ")).not.toContain("raw-secret-value");As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
🤖 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/llama-cpp-dgx-spark-qualification-runner.test.ts` around lines 263 - 264, Replace the vacuous raw-secret-value assertion in the test covering buildServerContainerArgv with a meaningful check: pass recognizable API-key material through an actual builder input and assert it is absent from argv, or remove that assertion and rely on the existing --api-key-file check. Keep the --api-key absence assertion intact.Source: Path instructions
scripts/checks/export-llama-cpp-image-config.mts (2)
218-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReject YAML warnings in both parsers, as the contract parser does.
parseQualificationRecipeandparseImageManifestcheck onlydocument.errors.parseActivationYamlinscripts/checks/llama-cpp-dgx-spark-qualification-contract.mtsrejects ondocument.errorsordocument.warnings. The recipe and the manifest are the two inputs that produce the hashed plan, so they deserve the same strictness. Add the warnings check to both parsers.♻️ Proposed strictness alignment
const document = YAML.parseDocument(source, { strict: true, uniqueKeys: true }); - if (document.errors.length > 0) { + if (document.errors.length > 0 || document.warnings.length > 0) { throw new Error(`invalid llama.cpp qualification recipe YAML: ${document.errors.join("; ")}`); }const document = YAML.parseDocument(source, { strict: true, uniqueKeys: true }); - if (document.errors.length > 0) { + if (document.errors.length > 0 || document.warnings.length > 0) { throw new Error(`invalid llama.cpp image manifest YAML: ${document.errors.join("; ")}`); }🤖 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 `@scripts/checks/export-llama-cpp-image-config.mts` around lines 218 - 244, Update parseQualificationRecipe and parseImageManifest to reject YAML documents when either document.errors or document.warnings is non-empty, matching parseActivationYaml’s strictness. Preserve the existing parser-specific error messages while including both diagnostics in the thrown errors.
828-844: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract one bounded regular-file reader. Both files define an identical
readBoundedRegularFilethat resolves a path against a root, rejects escapes, opens withO_NOFOLLOW, and bounds the size. The bodies differ only in the size limit and the error text.export-llama-cpp-dgx-spark-qualification-plan.mtsalready importsloadLlamaCppImageConfigFromRootfrom the other file, so one shared helper with amaximumBytesparameter and a caller-supplied label removes the copy and keeps the traversal and symlink controls in one place.
scripts/checks/export-llama-cpp-image-config.mts#L828-L844: exportreadBoundedRegularFileand accept the size limit and the error label as parameters.scripts/checks/export-llama-cpp-dgx-spark-qualification-plan.mts#L15-L31: delete the local copy and import the exported helper, passing 4096 and the protected-activation label.🤖 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 `@scripts/checks/export-llama-cpp-image-config.mts` around lines 828 - 844, Extract the duplicated bounded-file logic into the exported readBoundedRegularFile in scripts/checks/export-llama-cpp-image-config.mts: accept maximumBytes and a caller-supplied error label while preserving path traversal, O_NOFOLLOW, regular-file, and size checks. In scripts/checks/export-llama-cpp-dgx-spark-qualification-plan.mts, remove the local readBoundedRegularFile implementation, import the shared helper, and call it with 4096 and the protected-activation label.test/llama-cpp-dgx-spark-qualification-plan.test.ts (1)
100-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact error for duplicate activation keys.
Line 108 accepts any thrown error. The duplicate
profilekey makesparseActivationYamlthrow"llama.cpp DGX Spark activation YAML is invalid", which is a stable message. Assert it so the test proves duplicate-key rejection rather than any failure in the export path.♻️ Proposed assertion tightening
exportLlamaCppDgxSparkQualificationPlan( candidateRoot({ activation: `${activation}profile: dgx-spark-gb10-single\n`, enabled: true, }), ), - ).toThrow(); + ).toThrow("activation YAML is invalid");🤖 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/llama-cpp-dgx-spark-qualification-plan.test.ts` around lines 100 - 109, Update the duplicate-key test around exportLlamaCppDgxSparkQualificationPlan to assert the exact stable error message “llama.cpp DGX Spark activation YAML is invalid” rather than accepting any thrown error. Preserve the existing candidateRoot input and duplicate profile scenario.test/llama-cpp-dgx-spark-qualification-contract.test.ts (2)
272-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact error for the YAML alias case.
Line 276 accepts any thrown error. The neighbouring cases assert specific messages. An unrelated regression in
parseLlamaCppDgxSparkQualificationActivationwould still satisfy this assertion. Assert"activation YAML is invalid"so the test proves alias rejection specifically.♻️ Proposed assertion tightening
expect(() => parseLlamaCppDgxSparkQualificationActivation( `contractVersion: &version 1\njobId: ${LLAMA_CPP_DGX_SPARK_QUALIFICATION_JOB_ID}\nplatform: linux/arm64\nprofile: *version\n`, ), - ).toThrow(); + ).toThrow("activation YAML is invalid");🤖 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/llama-cpp-dgx-spark-qualification-contract.test.ts` around lines 272 - 276, Update the alias-rejection test for parseLlamaCppDgxSparkQualificationActivation to assert the exact error message "activation YAML is invalid" instead of accepting any thrown error, while preserving the existing YAML alias input and neighboring test structure.
373-384: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the canonicalization test to nested key order.
This test reorders only the three top-level keys. The producer in
scripts/checks/export-llama-cpp-image-config.mtscopiesrecipe.serve,recipe.policy,recipe.readiness,recipe.surfaces, andrecipe.model.fileverbatim from the recipe YAML, so their key order comes from the YAML author. A nested reordering case would prove thatllamaCppDgxSparkExecutionPlanSha256is stable against that input and would catch the producer-side digest coupling flagged onscripts/checks/export-llama-cpp-image-config.mts.♻️ Proposed additional nested-order case
it("canonicalizes execution plan field order before digest verification (`#8260`)", () => { const value = executionPlan(); const reordered = { recipe: value.recipe, imageBuild: value.imageBuild, contractVersion: value.contractVersion, }; expect(llamaCppDgxSparkExecutionPlanSha256(reordered)).toBe( llamaCppDgxSparkExecutionPlanSha256(value), ); + + const nestedReordered = { + ...value, + recipe: { + ...value.recipe, + model: { + ...value.recipe.model, + file: { + license: value.recipe.model.file.license, + quantization: value.recipe.model.file.quantization, + format: value.recipe.model.file.format, + sizeBytes: value.recipe.model.file.sizeBytes, + digest: value.recipe.model.file.digest, + path: value.recipe.model.file.path, + }, + }, + }, + }; + + expect(llamaCppDgxSparkExecutionPlanSha256(nestedReordered)).toBe( + llamaCppDgxSparkExecutionPlanSha256(value), + ); });🤖 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/llama-cpp-dgx-spark-qualification-contract.test.ts` around lines 373 - 384, Extend the test “canonicalizes execution plan field order before digest verification (`#8260`)” to reorder keys within nested execution-plan objects, including recipe.serve, recipe.policy, recipe.readiness, recipe.surfaces, and recipe.model.file. Assert that hashing the nested-reordered plan matches hashing the original, while preserving the existing top-level key-order coverage.test/llama-cpp-image.test.ts (1)
351-356: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact error for GPU-offload drift.
Line 356 accepts any thrown error.
loadLlamaCppImageConfigthrows from many sites, so a stale search string in thereplacecall would still satisfy this assertion through an unrelated error. Assert the recipe-contract message so the test proves that the offload drift is what fails.♻️ Proposed assertion tightening
expect(() => loadLlamaCppImageConfig( manifestSource, recipeSource.replace("offload: full", "offload: partial"), ), - ).toThrow(); + ).toThrow("invalid llama.cpp DGX Spark qualification recipe contract");🤖 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/llama-cpp-image.test.ts` around lines 351 - 356, Update the loadLlamaCppImageConfig assertion in the GPU-offload drift test to verify the specific recipe-contract error message, rather than accepting any thrown error. Keep the partial-offload mutation and ensure the assertion would fail if the replacement no longer changes the recipe as intended.
🤖 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 @.github/workflows/e2e.yaml:
- Line 2151: Update the job-level NEMOCLAW_LLAMA_CPP_QUALIFICATION_PLAN
definition to use a path under github.workspace instead of runner.temp, matching
the managed-image-multiarch-startup pattern. In the “Materialize trusted
llama.cpp qualification plan” step, create the plan’s parent directory before
writing it; keep the plan path separate from the receipt output path.
In `@scripts/checks/export-llama-cpp-image-config.mts`:
- Around line 668-691: Update the qualification plan digest generation in
exportLlamaCppDgxSparkQualificationPlan to use the canonical parse helper
parseLlamaCppDgxSparkExecutionPlan, hashing its reconstructed plan instead of
JSON.stringify on YAML-derived pass-through objects. Derive both exported plan
values from that canonical parse so producer and verifier agree regardless of
recipe key order, and remove the createHash import if no other code uses it.
In `@scripts/checks/run-llama-cpp-dgx-spark-qualification.mts`:
- Line 708: Update cleanupOwnedRuntime’s registryListenerClosed checks to
inspect port 5000 only when the qualification-owned registry container was
active and has been removed; do not treat an unrelated listener as cleanup
failure, including in the --cleanup-only path. Preserve the existing assertion
for listeners belonging to the owned registry runtime.
In `@test/e2e/support/llama-cpp-dgx-spark-qualification-workflow.test.ts`:
- Line 35: Update the suite title in describe("llama.cpp DGX Spark qualification
workflow boundary") to include the local issue reference as a final "(`#8260`)"
suffix, without adding the reference to child it titles.
In `@test/llama-cpp-dgx-spark-qualification-plan.test.ts`:
- Around line 70-71: Update the describe title for “llama.cpp DGX Spark
qualification plan export” to include the local issue reference as a final
“(`#8260`)” suffix; the child it titles are covered by this reference, so do not
add separate suffixes to them.
In `@tools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mts`:
- Around line 148-167: Add exact `uses === CHECKOUT` assertions for
`candidatePlanCheckout`, `trustedCheckout`, and `candidateCheckout`, matching
the existing `trustedPlanCheckout` check. Place each assertion after its
corresponding `requireStep` lookup so all remaining checkout steps reject
different full-SHA actions.
---
Nitpick comments:
In `@scripts/checks/export-llama-cpp-image-config.mts`:
- Around line 218-244: Update parseQualificationRecipe and parseImageManifest to
reject YAML documents when either document.errors or document.warnings is
non-empty, matching parseActivationYaml’s strictness. Preserve the existing
parser-specific error messages while including both diagnostics in the thrown
errors.
- Around line 828-844: Extract the duplicated bounded-file logic into the
exported readBoundedRegularFile in
scripts/checks/export-llama-cpp-image-config.mts: accept maximumBytes and a
caller-supplied error label while preserving path traversal, O_NOFOLLOW,
regular-file, and size checks. In
scripts/checks/export-llama-cpp-dgx-spark-qualification-plan.mts, remove the
local readBoundedRegularFile implementation, import the shared helper, and call
it with 4096 and the protected-activation label.
In `@scripts/checks/run-llama-cpp-dgx-spark-qualification.mts`:
- Around line 1067-1074: Update the top-level catch around main() to retain the
caught error and append its message when it is an Error instance, while
preserving the generic failure text for other thrown values. Keep the existing
stderr output and exitCode behavior unchanged.
- Around line 205-216: Remove the tautological registryOwner comparison from the
validation condition in the workflow check, since it compares
expectedRegistryOwner(runId, runAttempt) with itself. Keep the registryName
validation and simplify the surrounding logic so it clearly validates only the
effective registry-name constraint.
In `@test/llama-cpp-dgx-spark-qualification-contract.test.ts`:
- Around line 272-276: Update the alias-rejection test for
parseLlamaCppDgxSparkQualificationActivation to assert the exact error message
"activation YAML is invalid" instead of accepting any thrown error, while
preserving the existing YAML alias input and neighboring test structure.
- Around line 373-384: Extend the test “canonicalizes execution plan field order
before digest verification (`#8260`)” to reorder keys within nested execution-plan
objects, including recipe.serve, recipe.policy, recipe.readiness,
recipe.surfaces, and recipe.model.file. Assert that hashing the nested-reordered
plan matches hashing the original, while preserving the existing top-level
key-order coverage.
In `@test/llama-cpp-dgx-spark-qualification-plan.test.ts`:
- Around line 100-109: Update the duplicate-key test around
exportLlamaCppDgxSparkQualificationPlan to assert the exact stable error message
“llama.cpp DGX Spark activation YAML is invalid” rather than accepting any
thrown error. Preserve the existing candidateRoot input and duplicate profile
scenario.
In `@test/llama-cpp-dgx-spark-qualification-runner.test.ts`:
- Around line 263-264: Replace the vacuous raw-secret-value assertion in the
test covering buildServerContainerArgv with a meaningful check: pass
recognizable API-key material through an actual builder input and assert it is
absent from argv, or remove that assertion and rely on the existing
--api-key-file check. Keep the --api-key absence assertion intact.
In `@test/llama-cpp-image.test.ts`:
- Around line 351-356: Update the loadLlamaCppImageConfig assertion in the
GPU-offload drift test to verify the specific recipe-contract error message,
rather than accepting any thrown error. Keep the partial-offload mutation and
ensure the assertion would fail if the replacement no longer changes the recipe
as intended.
🪄 Autofix
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: 97b04ee1-a78f-4c4a-bce5-fe15fd07bceb
📒 Files selected for processing (24)
.github/workflows/e2e.yaml.github/workflows/llama-cpp-image.yamlmanaged-inference/images/llama-cpp/image.yamlscripts/checks/export-llama-cpp-dgx-spark-qualification-plan.mtsscripts/checks/export-llama-cpp-image-config.mtsscripts/checks/llama-cpp-dgx-spark-qualification-contract.mtsscripts/checks/run-llama-cpp-dgx-spark-qualification.mtstest/e2e/live/llama-cpp-dgx-spark-qualification-helpers.tstest/e2e/live/llama-cpp-dgx-spark-qualification.test.tstest/e2e/mock-parity.jsontest/e2e/support/llama-cpp-dgx-spark-qualification-workflow.test.tstest/helpers/vitest-watch-triggers.tstest/llama-cpp-dgx-spark-qualification-contract.test.tstest/llama-cpp-dgx-spark-qualification-plan.test.tstest/llama-cpp-dgx-spark-qualification-runner.test.tstest/llama-cpp-image.test.tstest/pr-risk-plan.test.tstest/vitest-watch-triggers.test.tstools/advisors/risk-plan.mtstools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mtstools/e2e/operations-workflow-boundary.mtstools/e2e/prepare-e2e-workflow-boundary.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mtstools/e2e/workflow-boundary.mts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
test/e2e/support/llama-cpp-dgx-spark-qualification-workflow.test.ts (2)
86-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest host networking and extra Buildx configuration independently.
buildx.withsets both an invalid driver andnetwork=host. This assertion proves only that the combined object is rejected. It does not prove that the validator rejects an extra Buildx option when the driver remains valid. Use separate fixtures for host networking and unexpected Buildx configuration.As per path instructions, review tests for behavioral confidence rather than implementation lock-in.
🤖 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/llama-cpp-dgx-spark-qualification-workflow.test.ts` around lines 86 - 98, Update the test named “rejects host networking or extra Buildx configuration” to use separate fixtures: one retaining the valid Docker driver while setting an unexpected Buildx option, and another covering host networking rejection. Assert each fixture against the appropriate validator result so both behaviors are independently verified without coupling the test to implementation details.Source: Path instructions
63-84: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the Git reference pin separately.
This test changes only
namedStep(...).usestoactions/checkout@main. The workflow contract also requires exactwith.refvalues for each checkout. Add cases that replace eachwith.refwith a mutable branch and assert rejection. Otherwise, a regression in the source commit pin can pass this test.As per path instructions, review tests for behavioral confidence rather than implementation lock-in.
🤖 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/llama-cpp-dgx-spark-qualification-workflow.test.ts` around lines 63 - 84, Extend the parameterized test around validateLlamaCppDgxSparkQualificationWorkflow to cover mutable with.ref values for each checkout step. For every relevant job/name pair, replace namedStep(...).with.ref with a branch such as main while keeping the checkout action pinned, then assert the validation error identifies the required reference; retain the existing uses-pin cases.Source: Path instructions
🤖 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.
Nitpick comments:
In `@test/e2e/support/llama-cpp-dgx-spark-qualification-workflow.test.ts`:
- Around line 86-98: Update the test named “rejects host networking or extra
Buildx configuration” to use separate fixtures: one retaining the valid Docker
driver while setting an unexpected Buildx option, and another covering host
networking rejection. Assert each fixture against the appropriate validator
result so both behaviors are independently verified without coupling the test to
implementation details.
- Around line 63-84: Extend the parameterized test around
validateLlamaCppDgxSparkQualificationWorkflow to cover mutable with.ref values
for each checkout step. For every relevant job/name pair, replace
namedStep(...).with.ref with a branch such as main while keeping the checkout
action pinned, then assert the validation error identifies the required
reference; retain the existing uses-pin cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2f56b317-6844-48d2-a959-7e2df194114d
📒 Files selected for processing (9)
.github/workflows/e2e.yamlscripts/checks/export-llama-cpp-image-config.mtsscripts/checks/run-llama-cpp-dgx-spark-qualification.mtstest/e2e/support/e2e-cross-runtime-compatibility.test.tstest/e2e/support/llama-cpp-dgx-spark-qualification-workflow.test.tstest/llama-cpp-dgx-spark-qualification-plan.test.tstest/llama-cpp-dgx-spark-qualification-runner.test.tstest/llama-cpp-image.test.tstools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/llama-cpp-dgx-spark-qualification-plan.test.ts
- tools/e2e/llama-cpp-dgx-spark-qualification-workflow-boundary.mts
- .github/workflows/e2e.yaml
- scripts/checks/run-llama-cpp-dgx-spark-qualification.mts
1 participant
Summary
Adds a dormant, protected qualification lane that can prove an exact NemoClaw-built llama.cpp ARM64 image candidate on one NVIDIA DGX Spark. The lane remains disabled until a follow-up YAML-only activation supplies the protected runner, approval environment, and verified local model path.
Related Issue
Part of #8260
Changes
maincode. A direct dynamic workflow binding is insufficient because candidate-controlled runner, environment, model-path, and command data must be rejected before protected work is scheduled; contract and plan-export tests cover the boundary.mainDockerfile and context in an isolated localhost registry. The candidate Dockerfile must byte-match trustedmain, Buildx cannot use host networking, and the runner verifies the pinned Nemotron GGUF, non-root one-GPU execution without egress, authenticated Chat Completions, full GPU-layer offload on NVIDIA GB10, and sanitized cleanup evidence.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX 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 validate:pr, semantic E2E phases, test-title style, and test-size checks pass.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: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests