fix(ci): reuse Hermes base image in boundary test - #8221
Conversation
📝 WalkthroughWalkthroughThe Hermes sandbox-image workflow resolves its base image before E2E tests. Validation requires exactly one canonical resolver before the secret-boundary probe. Tests cover missing, renamed, and misplaced resolver steps. ChangesHermes image validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 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 818de95 in the TypeScript / code-coverage/cliThe overall coverage in commit 818de95 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/support/sandbox-images-workflow-boundary.test.ts (1)
184-193: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover each resolver invariant in the regression tests.
This case only covers a missing named step. Add behavior cases for:
- a non-canonical
usesvalue;- a resolver placed after the secret-boundary step;
- more than one resolver invocation, including a duplicate with another step name.
As per path instructions, targeted checks should validate required action identity, ordering, and exact invocation count.
🤖 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/sandbox-images-workflow-boundary.test.ts` around lines 184 - 193, Extend the regression tests around validateSandboxImagesWorkflow to cover every Hermes base-image resolver invariant: reject a resolver with a non-canonical uses value, reject one placed after the secret-boundary probe, and reject multiple invocations including a duplicate under another step name. Ensure each case asserts the expected validation error for canonical action identity, ordering, and exactly one invocation.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 `@tools/e2e/sandbox-images-workflow-boundary.mts`:
- Around line 907-920: Update the validation around baseImageResolvers to count
every workflow step whose uses value equals HERMES_BASE_IMAGE_RESOLVER_ACTION,
regardless of step name. Require exactly one such action step, and ensure that
step is named "Resolve Hermes base image" and runs before secretBoundary;
preserve the existing validation error behavior.
---
Nitpick comments:
In `@test/e2e/support/sandbox-images-workflow-boundary.test.ts`:
- Around line 184-193: Extend the regression tests around
validateSandboxImagesWorkflow to cover every Hermes base-image resolver
invariant: reject a resolver with a non-canonical uses value, reject one placed
after the secret-boundary probe, and reject multiple invocations including a
duplicate under another step name. Ensure each case asserts the expected
validation error for canonical action identity, ordering, and exactly one
invocation.
🪄 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: d2b5ed72-4a4e-46ce-8e77-80f395798d2b
📒 Files selected for processing (3)
.github/workflows/sandbox-images-and-e2e.yamltest/e2e/support/sandbox-images-workflow-boundary.test.tstools/e2e/sandbox-images-workflow-boundary.mts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 terminology difference from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
5 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 2 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. |
1 participant
Summary
The Hermes secret-boundary consumer rebuilt the full Hermes base image when it prepared the managed variant. The repeated build exceeded the probe's 10-minute process limit on
main, so the consumer now resolves the verified base image before the live probe.Changes
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 unavailablenpx vitest run --project e2e-support test/e2e/support/sandbox-images-workflow-boundary.test.ts test/e2e/support/hermes-secret-boundary-workflow.test.ts— 35 tests passed.npm run checks:repositoryand focused Biome checks also passed.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: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit