ci(e2e): reuse exact-commit CLI artifact - #7943
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@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. |
📝 WalkthroughWalkthroughThe E2E workflow now builds one exact-commit CLI artifact, records provenance, and restores it across consumer jobs. A composite action validates identity, integrity, archive safety, and build metadata. Workflow validators and tests enforce producer, consumer, and ordering contracts. ChangesCLI artifact reuse
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GenerateMatrix as generate-matrix
participant ArtifactStore as GitHub artifact store
participant RestoreAction as restore-e2e-cli-artifact
participant E2EJob as E2E consumer job
GenerateMatrix->>GenerateMatrix: build and record provenance
GenerateMatrix->>ArtifactStore: upload CLI artifact
E2EJob->>RestoreAction: pass cli_artifact_provenance
RestoreAction->>ArtifactStore: download and verify artifact
RestoreAction->>E2EJob: restore dist and verify CLI version
E2EJob->>E2EJob: run E2E tests
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 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: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
tools/e2e/runner-comparison-workflow-boundary.mts (1)
197-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecord why the
preparefallback stays.
bootstrapEndfalls back topreparewhen no restore step exists. That fallback is load-bearing, not defensive:security-postureis a comparison job and also a member ofPREPARE_E2E_NO_BUILD_JOBS, so it never restores the artifact. Without a comment, a later cleanup can remove the fallback and break that job's validation. Add one line that names the case.📝 Proposed comment
+ // Comparison jobs that never build or restore the CLI (for example + // security-posture, a PREPARE_E2E_NO_BUILD_JOBS member) end bootstrap at + // prepare-e2e, so the restore step is optional here. const restore = jobSteps.findIndex((step) => step.name === CLI_ARTIFACT_RESTORE_STEP); const bootstrapEnd = restore >= 0 ? restore : prepare;🤖 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 `@tools/e2e/runner-comparison-workflow-boundary.mts` around lines 197 - 198, Add a concise inline comment immediately above the `bootstrapEnd` assignment explaining that comparison jobs such as `security-posture` skip artifact restoration because they belong to `PREPARE_E2E_NO_BUILD_JOBS`, so the `prepare` fallback must be preserved.test/e2e/support/rebuild-hermes-workflow-boundary.test.ts (1)
60-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case that removes only the restore step.
The suite never asserts the missing-restore-step error.
validateRebuildHermesBootstrapBoundaryguards its ordering check withrestoreCli &&, so the ordering check short-circuits when the restore step is absent. If therequireJobStepcall forCLI_ARTIFACT_RESTORE_STEPis removed later, a job that never restores the artifact produces no error and this suite still passes.The existing negative case mutates the environment, the
withblock, and the step order together, so it cannot cover this path. Add one focused case that drops only the restore step.As per path instructions: "Migration tests must prove the superseded path is unreachable or removed, not merely prove that the new path also works."
💚 Proposed additional case
it.each(JOB_NAMES)("%s must require the exact-commit CLI restore step", (jobName) => { const job = bootstrapJob(jobName); job.steps = job.steps.filter((step) => step.name !== "Restore exact-commit CLI artifact"); expect(validateRebuildHermesBootstrapBoundary(jobName, job)).toContain( `${jobName} job is missing step 'Restore exact-commit CLI artifact'`, ); });Match the expected string to the exact text that
requireJobStepproduces.🤖 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/rebuild-hermes-workflow-boundary.test.ts` around lines 60 - 78, Add a focused parameterized test alongside the existing rebuild Hermes boundary cases that removes only the step named “Restore exact-commit CLI artifact” from the job returned by bootstrapJob. Assert validateRebuildHermesBootstrapBoundary reports the missing CLI_ARTIFACT_RESTORE_STEP using the exact error text produced by requireJobStep, without changing environment, preparation, installation, or ordering fields.Source: Path instructions
test/e2e/support/cli-artifact-workflow-boundary.test.ts (2)
418-424: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the temporary directory after the assertion.
Line 419 creates a directory under
os.tmpdir()and the test never removes it. Every run leaks one directory. The other fixtures in this file clean up throughcleanup()in afinallyblock. Apply the same pattern here.♻️ Proposed fix
it("rejects action implementation drift that weakens extraction or payload verification", () => { const directory = fs.mkdtempSync(path.join(os.tmpdir(), "cli-artifact-action-")); - const actionPath = path.join(directory, "action.yaml"); - const source = readRepoText(".github/actions/restore-e2e-cli-artifact/action.yaml") - .replace("tar --no-same-owner --no-same-permissions", "tar") - .replace('[[ "$actual_payload_sha256" == "$PAYLOAD_SHA256" ]]', '[[ -s "$payload" ]]'); - fs.writeFileSync(actionPath, source); - - expect(validateCliArtifactRestoreAction(actionPath)).toEqual( - expect.arrayContaining([ - "CLI artifact restore action must match its immutable workflow pin", - 'CLI artifact payload verification must contain tar --no-same-owner --no-same-permissions -xf "$payload" -C "$restore_dir"', - 'CLI artifact payload verification must contain [[ "$actual_payload_sha256" == "$PAYLOAD_SHA256" ]]', - ]), - ); + try { + const actionPath = path.join(directory, "action.yaml"); + const source = readRepoText(".github/actions/restore-e2e-cli-artifact/action.yaml") + .replace("tar --no-same-owner --no-same-permissions", "tar") + .replace('[[ "$actual_payload_sha256" == "$PAYLOAD_SHA256" ]]', '[[ -s "$payload" ]]'); + fs.writeFileSync(actionPath, source); + + expect(validateCliArtifactRestoreAction(actionPath)).toEqual( + expect.arrayContaining([ + "CLI artifact restore action must match its immutable workflow pin", + 'CLI artifact payload verification must contain tar --no-same-owner --no-same-permissions -xf "$payload" -C "$restore_dir"', + 'CLI artifact payload verification must contain [[ "$actual_payload_sha256" == "$PAYLOAD_SHA256" ]]', + ]), + ); + } finally { + fs.rmSync(directory, { force: true, recursive: true }); + } });🤖 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/cli-artifact-workflow-boundary.test.ts` around lines 418 - 424, Update the test case around “rejects action implementation drift that weakens extraction or payload verification” to remove its temporary directory after the assertion, following the file’s existing cleanup() pattern in a finally block and ensuring cleanup runs on both success and failure.
287-298: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the rejection reason for each malformed provenance case.
This table asserts only a non-zero exit status. A bash syntax error or an unrelated failure in the identity script satisfies every row. The neighbouring table on lines 300-313 already pairs each case with its expected message. Apply the same shape here so each row proves the specific guard it names.
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/cli-artifact-workflow-boundary.test.ts` around lines 287 - 298, Update the malformed provenance cases in the it.each table to include each case’s expected rejection message, then assert both non-zero status and the corresponding message from runIdentityValidation. Match the neighboring table’s assertion shape so failures prove the specific validation guard rather than an unrelated script error.Source: Path instructions
tools/e2e/cli-artifact-workflow-boundary.mts (1)
76-105: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the validator fail with an error message instead of throwing.
Line 76 reads the action file without a guard. If
.github/actions/restore-e2e-cli-artifact/action.yamlis missing or renamed,validateCliArtifactRestoreActionthrows.validateE2eWorkflowintools/e2e/workflow-boundary.mtsthen aborts before it collects the remaining boundary errors.Line 97 also mixes access styles. Line 96 uses
identity?.name, then line 97 usesidentity.id. A YAML list entry that parses tonullmakes line 97 throw aTypeError. The same pattern applies todownload.useson line 123.♻️ Proposed fix
const errors: string[] = []; - const actionSource = readFileSync(actionPath, "utf8"); + let actionSource: string; + try { + actionSource = readFileSync(actionPath, "utf8"); + } catch { + return ["CLI artifact restore action file is missing or unreadable"]; + }if ( identity?.name !== "Validate exact-commit CLI artifact identity" || - identity.id !== "identity" || - identity.shell !== "bash" || + identity?.id !== "identity" || + identity?.shell !== "bash" ||if ( download?.name !== CLI_ARTIFACT_DOWNLOAD_STEP || - download.uses !== CLI_ARTIFACT_DOWNLOAD_ACTION || + download?.uses !== CLI_ARTIFACT_DOWNLOAD_ACTION ||🤖 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 `@tools/e2e/cli-artifact-workflow-boundary.mts` around lines 76 - 105, Update validateCliArtifactRestoreAction to catch missing or unreadable action files and append a validation error instead of throwing, allowing validateE2eWorkflow to continue collecting other boundary errors. Also make all destructured action-step property checks null-safe, including identity.id and download.uses, while preserving the existing validation messages and behavior for valid entries.
🤖 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/actions/restore-e2e-cli-artifact/action.yaml:
- Around line 159-173: Update the preexisting-dist guard before restore_dir in
the restore action to reject any existing $GITHUB_WORKSPACE/dist entry,
including dangling symlinks, by checking both existence and symlink status
rather than relying on -e alone. Preserve the current error-and-exit behavior,
and update any matching pinned fragments in
tools/e2e/cli-artifact-workflow-boundary.mts.
In `@test/e2e/README.md`:
- Around line 34-35: Clarify the README wording by replacing the ambiguous “It”
in the sentence following “Each consumer” with the explicit consumer job
subject, or combine both sentences so the setting clearly applies to each
consumer. Preserve the existing build-cli value and preparation-action details.
In `@tools/e2e/cli-artifact-workflow-boundary.mts`:
- Around line 332-334: Update the missing-producer branch in the workflow
validation logic to append its message to the existing errors collection instead
of returning a new array. Preserve the restore-action drift errors gathered
before the Object.keys(producer) check, then return errors after adding the
missing CLI artifact producer message.
---
Nitpick comments:
In `@test/e2e/support/cli-artifact-workflow-boundary.test.ts`:
- Around line 418-424: Update the test case around “rejects action
implementation drift that weakens extraction or payload verification” to remove
its temporary directory after the assertion, following the file’s existing
cleanup() pattern in a finally block and ensuring cleanup runs on both success
and failure.
- Around line 287-298: Update the malformed provenance cases in the it.each
table to include each case’s expected rejection message, then assert both
non-zero status and the corresponding message from runIdentityValidation. Match
the neighboring table’s assertion shape so failures prove the specific
validation guard rather than an unrelated script error.
In `@test/e2e/support/rebuild-hermes-workflow-boundary.test.ts`:
- Around line 60-78: Add a focused parameterized test alongside the existing
rebuild Hermes boundary cases that removes only the step named “Restore
exact-commit CLI artifact” from the job returned by bootstrapJob. Assert
validateRebuildHermesBootstrapBoundary reports the missing
CLI_ARTIFACT_RESTORE_STEP using the exact error text produced by requireJobStep,
without changing environment, preparation, installation, or ordering fields.
In `@tools/e2e/cli-artifact-workflow-boundary.mts`:
- Around line 76-105: Update validateCliArtifactRestoreAction to catch missing
or unreadable action files and append a validation error instead of throwing,
allowing validateE2eWorkflow to continue collecting other boundary errors. Also
make all destructured action-step property checks null-safe, including
identity.id and download.uses, while preserving the existing validation messages
and behavior for valid entries.
In `@tools/e2e/runner-comparison-workflow-boundary.mts`:
- Around line 197-198: Add a concise inline comment immediately above the
`bootstrapEnd` assignment explaining that comparison jobs such as
`security-posture` skip artifact restoration because they belong to
`PREPARE_E2E_NO_BUILD_JOBS`, so the `prepare` fallback must be preserved.
🪄 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: 9f3938a2-7881-46df-8265-819d238da987
📒 Files selected for processing (13)
.github/actions/restore-e2e-cli-artifact/action.yaml.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/support/cli-artifact-workflow-boundary.test.tstest/e2e/support/prepare-e2e-workflow-boundary.test.tstest/e2e/support/rebuild-hermes-workflow-boundary.test.tstest/e2e/support/runner-comparison-workflow-boundary.test.tstools/e2e/cli-artifact-workflow-boundary.mtstools/e2e/hermes-gpu-startup-workflow-boundary.mtstools/e2e/prepare-e2e-workflow-boundary.mtstools/e2e/runner-comparison-workflow-boundary.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mtstools/e2e/workflow-boundary.mts
| Each consumer still runs the pinned preparation action for Node.js and dependency installation. | ||
| It sets `build-cli: "false"`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the ambiguous pronoun.
Line 35 starts with "It". The nearest noun on line 34 is "the pinned preparation action", but the intended subject is the consumer job. Name the subject explicitly, or join the two sentences.
📝 Proposed wording
-Each consumer still runs the pinned preparation action for Node.js and dependency installation.
-It sets `build-cli: "false"`.
+Each consumer still runs the pinned preparation action for Node.js and dependency installation.
+Each consumer sets `build-cli: "false"` on that action.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Each consumer still runs the pinned preparation action for Node.js and dependency installation. | |
| It sets `build-cli: "false"`. | |
| Each consumer still runs the pinned preparation action for Node.js and dependency installation. | |
| Each consumer sets `build-cli: "false"` on that action. |
🤖 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/README.md` around lines 34 - 35, Clarify the README wording by
replacing the ambiguous “It” in the sentence following “Each consumer” with the
explicit consumer job subject, or combine both sentences so the setting clearly
applies to each consumer. Preserve the existing build-cli value and
preparation-action details.
| if (Object.keys(producer).length === 0) { | ||
| return [`workflow is missing CLI artifact producer ${CLI_ARTIFACT_PRODUCER_JOB}`]; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the restore-action errors when the producer job is missing.
Line 329 collects restore-action drift errors. Line 333 returns a new array and discards them. If a contributor removes generate-matrix and also weakens the composite action, the report names only the missing producer. Push the message onto errors and return errors.
🐛 Proposed fix
if (Object.keys(producer).length === 0) {
- return [`workflow is missing CLI artifact producer ${CLI_ARTIFACT_PRODUCER_JOB}`];
+ errors.push(`workflow is missing CLI artifact producer ${CLI_ARTIFACT_PRODUCER_JOB}`);
+ return errors;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Object.keys(producer).length === 0) { | |
| return [`workflow is missing CLI artifact producer ${CLI_ARTIFACT_PRODUCER_JOB}`]; | |
| } | |
| if (Object.keys(producer).length === 0) { | |
| errors.push(`workflow is missing CLI artifact producer ${CLI_ARTIFACT_PRODUCER_JOB}`); | |
| return errors; | |
| } |
🤖 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 `@tools/e2e/cli-artifact-workflow-boundary.mts` around lines 332 - 334, Update
the missing-producer branch in the workflow validation logic to append its
message to the existing errors collection instead of returning a new array.
Preserve the restore-action drift errors gathered before the
Object.keys(producer) check, then return errors after adding the missing CLI
artifact producer message.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
.github/actions/restore-e2e-cli-artifact/action.yaml (2)
174-174: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCompare the CLI version with the build identity
bin/nemoclaw.jsdelegates to../dist/nemoclaw, so this check exercises the restored artifact. Compare the reported version with.nemoclawVersionfromdist/build-identity.json.🤖 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 @.github/actions/restore-e2e-cli-artifact/action.yaml at line 174, Update the CLI validation around bin/nemoclaw.js to capture its reported version and compare it against the .nemoclawVersion value from dist/build-identity.json, failing the action when they differ; retain the restored-artifact check while suppressing only the normal version output.
74-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBind
artifactDigestto the download, or remove the dead output.
artifact_digestis written at line 75, but no later step reads it.digest-mismatch: errorcompares the downloaded artifact with the digest reported by the GitHub API, not with the provenanceartifactDigest. Compare both digests, or removeartifact_digestfromGITHUB_OUTPUT. Update the full-content SHA and action commit pin if the action changes.🤖 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 @.github/actions/restore-e2e-cli-artifact/action.yaml around lines 74 - 91, Update the artifact download flow after the provenance parsing to use steps.identity.outputs.artifact_digest in an explicit comparison with the downloaded artifact’s digest, while retaining the existing GitHub API digest validation; alternatively remove the artifact_digest output if no comparison is implemented. If changing the download action, update its full-content SHA and version comment.test/e2e/support/cli-artifact-workflow-boundary.test.ts (2)
32-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTemporary directories are created without a matching removal. Two helpers call
fs.mkdtempSyncand never remove the directory.runRestoreValidationalready returns acleanuphandle for this purpose. Apply the same pattern to both sites so a full test run leaves no directories in the temporary directory.
test/e2e/support/cli-artifact-workflow-boundary.test.ts#L32-L59: remove theoutputDirectorycreated at line 35 in afinallyblock. This helper runs in 13 test cases.test/e2e/support/cli-artifact-workflow-boundary.test.ts#L418-L433: remove thedirectorycreated at line 419 in afinallyblock around theexpectcall.🤖 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/cli-artifact-workflow-boundary.test.ts` around lines 32 - 59, Update runIdentityValidation in test/e2e/support/cli-artifact-workflow-boundary.test.ts lines 32-59 to return or use a cleanup handle and remove outputDirectory in a finally block for every invocation; also update the helper/test at test/e2e/support/cli-artifact-workflow-boundary.test.ts lines 418-433 to remove directory in a finally block around the expect call, following the existing runRestoreValidation cleanup pattern.
36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSelect action steps by identity, not array position.
runIdentityValidationusessteps[0], whilerunRestoreValidationusessteps[2]. Select the first step byidand the restore step by its uniquename, then assert that each step exists.🤖 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/cli-artifact-workflow-boundary.test.ts` at line 36, Update runIdentityValidation and runRestoreValidation to locate action steps by identity rather than array position: find the first step by its id and the restore step by its unique name, then assert each lookup succeeds before accessing its run command.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 @.github/actions/restore-e2e-cli-artifact/action.yaml:
- Line 174: Update the CLI validation around bin/nemoclaw.js to capture its
reported version and compare it against the .nemoclawVersion value from
dist/build-identity.json, failing the action when they differ; retain the
restored-artifact check while suppressing only the normal version output.
- Around line 74-91: Update the artifact download flow after the provenance
parsing to use steps.identity.outputs.artifact_digest in an explicit comparison
with the downloaded artifact’s digest, while retaining the existing GitHub API
digest validation; alternatively remove the artifact_digest output if no
comparison is implemented. If changing the download action, update its
full-content SHA and version comment.
In `@test/e2e/support/cli-artifact-workflow-boundary.test.ts`:
- Around line 32-59: Update runIdentityValidation in
test/e2e/support/cli-artifact-workflow-boundary.test.ts lines 32-59 to return or
use a cleanup handle and remove outputDirectory in a finally block for every
invocation; also update the helper/test at
test/e2e/support/cli-artifact-workflow-boundary.test.ts lines 418-433 to remove
directory in a finally block around the expect call, following the existing
runRestoreValidation cleanup pattern.
- Line 36: Update runIdentityValidation and runRestoreValidation to locate
action steps by identity rather than array position: find the first step by its
id and the restore step by its unique name, then assert each lookup succeeds
before accessing its run command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9bc88616-b973-41d5-9feb-771a216430da
📒 Files selected for processing (13)
.github/actions/restore-e2e-cli-artifact/action.yaml.github/workflows/e2e.yamltest/e2e/README.mdtest/e2e/support/cli-artifact-workflow-boundary.test.tstest/e2e/support/prepare-e2e-workflow-boundary.test.tstest/e2e/support/rebuild-hermes-workflow-boundary.test.tstest/e2e/support/runner-comparison-workflow-boundary.test.tstools/e2e/cli-artifact-workflow-boundary.mtstools/e2e/hermes-gpu-startup-workflow-boundary.mtstools/e2e/prepare-e2e-workflow-boundary.mtstools/e2e/runner-comparison-workflow-boundary.mtstools/e2e/upload-e2e-artifacts-workflow-boundary.mtstools/e2e/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (10)
- test/e2e/support/runner-comparison-workflow-boundary.test.ts
- tools/e2e/upload-e2e-artifacts-workflow-boundary.mts
- test/e2e/support/prepare-e2e-workflow-boundary.test.ts
- tools/e2e/runner-comparison-workflow-boundary.mts
- tools/e2e/workflow-boundary.mts
- tools/e2e/prepare-e2e-workflow-boundary.mts
- .github/workflows/e2e.yaml
- tools/e2e/cli-artifact-workflow-boundary.mts
- test/e2e/support/rebuild-hermes-workflow-boundary.test.ts
- tools/e2e/hermes-gpu-startup-workflow-boundary.mts
wscurran
added
area: ci
4 participants
Summary
Build the candidate CLI once in
generate-matrix, publish a content-addressed artifact, and restore it in 63 live E2E consumers. The restore contract is pinned to an immutable commit and fails closed on provenance, digest, archive, candidate-SHA, compiled build-identity, or activation mismatch.This branch is current with
main; the retiredsandbox-rebuildandupgrade-stale-sandboxjobs remain retired.Related Issue
Advances #7915.
The before/after wall-time and runner-minute acceptance criterion remains open. Trusted PR E2E executes
.github/workflows/e2e.yamlfrommain, so an honest post-change comparison requires a passing post-mergemainrun.Changes
generate-matrixthe single default CLI producer and pass one closed-schemanemoclaw-e2e-cli-provenance-v1object to 63 restore consumers.e5a55a8be89d4a3dfd44b743c7190544ef2f5246.dist/build-identity.jsonto bind the compiled CLI to the candidate commit before activation.distmembers, traversal, links, special files, or a preexisting workspacedist/.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedtest/e2e/README.md; focused artifact suite 27/27; full E2E-support suite 1,871 passed and 17 skipped; test-conditionals scan, Biome, Markdown lint, andgit diff --checkpassedDGX 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 docsbuilds without warnings (doc changes only)Signed-off-by: Charan Jagwani cjagwani@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Tests