test(e2e): define protected inference qualification - #8079
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 WalkthroughWalkthroughAdds a dormant native-runtime activation qualification contract. It compiles a provider-neutral 24-case matrix, validates protected execution evidence, supplies Podman fixtures, adds fail-closed tests, and documents the activation boundary. ChangesNative runtime qualification
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ericksoa
added
chore
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 326edb5 in the Show a code coverage summary of the most impacted files.
TypeScript / code-coverage/cliThe overall coverage in commit 326edb5 in the Show a code coverage summary of the most impacted files.
Updated |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/e2e/support/e2e-native-runtime-qualification.test.ts (1)
339-347: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSelect the case by agent instead of by index.
completeEvidence()[0]depends on the compiled sort order. The first id ispodman-dcode-..., so"hermes"is currently a mismatched application. If the sort order or the agent list changes, the value can become the correct application, and the test passes without exercising the check. Find the case by agent, then set an application that belongs to a different agent.🤖 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/e2e-native-runtime-qualification.test.ts` around lines 339 - 347, Update the test around completeEvidence to locate the target runtime evidence by its agent identifier instead of assuming index 0, then set its application to a value belonging to a different agent. Keep the assertion against assertNativeRuntimeQualificationEvidence and preserve the invalid runtime identity expectation.test/e2e/registry/activation-qualification.ts (1)
565-651: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider splitting
assertCaseEvidenceinto focused validators.The function validates protected-run, installer, runtime identity, engine authority, managed images, route, lifecycle, and CDI evidence in one body. The neighboring helpers
assertEngineAuthority,assertInferenceRoute, andassertLifecycleEvidencealready show the pattern. ExtractassertProtectedRun,assertInstallerReceipt, andassertRuntimeIdentityto keep function complexity low.As per coding guidelines: "Keep function complexity low and prefix intentionally unused variables with
_".🤖 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/registry/activation-qualification.ts` around lines 565 - 651, Split assertCaseEvidence into focused helpers for protected-run validation, installer receipt validation, and runtime identity validation, following the existing assertEngineAuthority, assertInferenceRoute, and assertLifecycleEvidence pattern. Move each corresponding validation block into assertProtectedRun, assertInstallerReceipt, and assertRuntimeIdentity, then invoke them from assertCaseEvidence while preserving all checks and error behavior; prefix any intentionally unused parameters or variables with an underscore.Source: Coding guidelines
🤖 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/registry/activation-qualification.ts`:
- Around line 251-268: Update qualificationCaseId to normalize only
input.acceleration before assembling the identifier, mapping the nvidia-gpu
acceleration token to gpu while leaving provider and other tokens unchanged.
Remove the joined-string replace so provider values such as nvidia-gpu-native
cannot be altered or produce duplicate IDs.
In `@test/e2e/support/e2e-native-runtime-qualification.test.ts`:
- Around line 164-175: Update the obligations assertion in the complete-case
coverage test to include the missing "agent.onboard" obligation and compare the
collection exactly rather than using expect.arrayContaining, ensuring all and
only the nine expected obligations are required.
---
Nitpick comments:
In `@test/e2e/registry/activation-qualification.ts`:
- Around line 565-651: Split assertCaseEvidence into focused helpers for
protected-run validation, installer receipt validation, and runtime identity
validation, following the existing assertEngineAuthority, assertInferenceRoute,
and assertLifecycleEvidence pattern. Move each corresponding validation block
into assertProtectedRun, assertInstallerReceipt, and assertRuntimeIdentity, then
invoke them from assertCaseEvidence while preserving all checks and error
behavior; prefix any intentionally unused parameters or variables with an
underscore.
In `@test/e2e/support/e2e-native-runtime-qualification.test.ts`:
- Around line 339-347: Update the test around completeEvidence to locate the
target runtime evidence by its agent identifier instead of assuming index 0,
then set its application to a value belonging to a different agent. Keep the
assertion against assertNativeRuntimeQualificationEvidence and preserve the
invalid runtime identity expectation.
🪄 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: fcfa10da-ed0a-4314-b8e8-a2408df85136
📒 Files selected for processing (5)
test/e2e/docs/NATIVE_RUNTIME_QUALIFICATION.mdtest/e2e/docs/README.mdtest/e2e/registry/activation-qualification.tstest/e2e/support/e2e-native-runtime-qualification.test.tstest/e2e/support/native-runtime-qualification-fixtures.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
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. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/support/e2e-native-runtime-qualification.test.ts (1)
403-419: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the frozen snapshot directly to complete the freeze claim.
Line 408 mutates the test-local
evidenceobject. The reporter already holds an independentstructuredClone, so this proves snapshot independence. It does not prove freezing. A clone withoutdeepFreezewould produce the same passing result, because nothing writes to the reporter's copy.Add an assertion that a write to the reporter-held evidence fails. This exercises the second half of the test title.
♻️ Proposed addition
evidence[0]!.installer.invocation.sha256 = "e".repeat(64); + expect(() => { + (evidence as NativeRuntimeQualificationEvidence[])[0]!.protectedRun.runId = 1; + }).not.toThrow(); writeEvidenceArtifacts(materialized.artifactRoot, completeEvidence());Replace the placeholder above with a check against the frozen copy that the reporter exposes. If the reporter does not expose its evidence, add a narrow accessor in
test/e2e/registry/activation-qualification.ts, or rename the test to claim independence only.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/e2e/support/e2e-native-runtime-qualification.test.ts` around lines 403 - 419, The test verifies snapshot independence but not that the reporter’s stored evidence is frozen. In the test around createNativeRuntimeQualificationReporterRecord, assert that mutating the reporter-held evidence throws; if that evidence is not exposed, add a narrow accessor in the reporter implementation, or rename the test to claim independence only.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.
Inline comments:
In `@test/e2e/registry/activation-qualification.ts`:
- Around line 594-596: Update protectedRunKey to construct its serialized key
using the explicit eight-field canonical order rather than raw-object
JSON.stringify. In test/e2e/registry/activation-qualification.ts lines 658-668,
normalize entry.protectedRun with exactRecord using the same eight-key list as
line 632 before passing it to protectedRunKey; apply both changes so binding and
evidence lookups use identical normalization.
---
Nitpick comments:
In `@test/e2e/support/e2e-native-runtime-qualification.test.ts`:
- Around line 403-419: The test verifies snapshot independence but not that the
reporter’s stored evidence is frozen. In the test around
createNativeRuntimeQualificationReporterRecord, assert that mutating the
reporter-held evidence throws; if that evidence is not exposed, add a narrow
accessor in the reporter implementation, or rename the test to claim
independence only.
🪄 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: 84e4f9e8-bd31-4257-a6e5-f7d424ceb049
📒 Files selected for processing (3)
test/e2e/docs/NATIVE_RUNTIME_QUALIFICATION.mdtest/e2e/registry/activation-qualification.tstest/e2e/support/e2e-native-runtime-qualification.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
1 participant
Summary
amd64/arm64, CPU Ollama, and GPU Ollama/NIM/vLLMStack identity
feat/host-local-inference-lifecycle-recovery75cd0af36c2be335f24603c7e612237ed0f10c58326edb502fc6f835b151b0fcf4e1681f1e8bf4865ab28f42a69ab03f12dc4eac8bcb257a3c69b34dQualification contract
The compiler fails closed unless the candidate declares all of the following:
amd64andarm64The compiler is keyed by an open provider ID. The MXC-style contract test demonstrates that another provider can plug in without a provider-name switch in central orchestration.
Inert boundary
This PR does not register or activate Podman, add a live target or workflow, change installer selection, or advertise runtime support. It intentionally contains no unconsumed protected-collector API. The follow-on collector slice must introduce authenticated run/job bindings, artifact-byte verification, and final evidence acceptance together with the protected workflow that consumes them.
No requirement was removed from the activation target. The full reporter implementation remains preserved in signed source history and on the durable follow-on backup branch for reconstruction with its consumer.
Source extraction
The qualification-only source material came from slice-pure commits
eeee3f92bandd70120a47. No cross-cutting restack merge or activation commit was cherry-picked.Validation
Related to #7744.
Signed-off-by: Aaron Erickson aerickson@nvidia.com