fix(e2e): load private file helper as ESM - #7229
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds protected private-file helpers in ChangesPrivate file handling and E2E migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit b4c0214 in the TypeScript / code-coverage/cliThe overall coverage in commit b4c0214 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
🤖 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-private-file.test.ts`:
- Around line 15-25: Move the live tsx process assertion from the root-level
test into the test/e2e suite, preserving the spawnSync invocation of
tools/e2e/live-test-outcome.mts and its real process-boundary validation. Remove
the root test’s internal private-file helper import and keep root-level tests
limited to CLI source imports.
🪄 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: 4e341b46-ec72-477d-ab40-0df22b81807d
📒 Files selected for processing (7)
test/e2e-private-file.test.tstest/e2e/risk-signal-reporter.tstest/pr-risk-plan.test.tstools/e2e/live-test-outcome.mtstools/e2e/pr-e2e-gate.mtstools/e2e/private-file.mtstools/e2e/runner-pressure.mts
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: Blockers
|
Signed-off-by: San Dang <sdang@nvidia.com>
E2E Target Results — ❌ Some tests failedRun: 29732601939
|
Signed-off-by: San Dang <sdang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/support/rebuild-hermes-progress.test.ts`:
- Around line 51-52: Update the fake Docker script generation in
rebuild-hermes-progress.test.ts to avoid embedding dockerMarker directly in
shell source. Pass the marker through a safely quoted environment variable or
derive it from the script path, while preserving the existing marker-touch
behavior when the generated docker executable runs.
🪄 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: 3e26fdca-07eb-4555-839b-4726fe8463d4
📒 Files selected for processing (3)
test/e2e/live/rebuild-hermes-progress.tstest/e2e/support/rebuild-hermes-progress.test.tstools/e2e/runner-pressure.mts
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
E2E Target Results — ❌ Some tests failedRun: 29744383264
|
E2E Target Results — ❌ Some tests failedRun: 29750575389
|
Signed-off-by: San Dang <sdang@nvidia.com>
E2E Target Results — ✅ All selected tests passedRun: 29751393619
|
E2E Target Results — ❌ Some tests failedRun: 29751393624
|
E2E Target Results — ❌ Some tests failedRun: 29751740423
|
E2E Target Results — ✅ All requested tests passedRun: 29744383264
|
E2E Target Results — ❌ Some tests failedRun: 29749845209
|
Signed-off-by: San Dang <sdang@nvidia.com>
E2E Target Results — ❌ Some tests failedRun: 29753559103
|
E2E Target Results — ❌ Some tests failedRun: 29753559103
|
E2E Target Results — ❌ Some tests failedRun: 29758219713
|
wscurran
added
area: ci
E2E Target Results — ❌ Some tests failedRun: 29758219713
|
|
Closing as superseded by #7217, which already merged the original private-file ESM loader fix into main. Any remaining distinct follow-up changes can be proposed separately if still needed. |
3 participants
Summary
The scheduled Hermes rebuild jobs failed before their tests started because an
.mtsentrypoint imported named exports from a.tshelper thattsxclassified as CommonJS. After the ESM correction let the live jobs proceed, their minute-by-minute Docker inspection blocked behind the large BuildKit export, exhausted the hosted runners, and canceled both lanes. This change makes the helper explicitly ESM, protects the production invocation boundary, and keeps in-flight rebuild heartbeats on host-only resource collection while preserving Docker evidence at the workflow baseline and terminal classifier.Changes
tools/e2e/private-file.tstotools/e2e/private-file.mtswithout changing its file-safety logic.npx tsx tools/e2e/live-test-outcome.mtsentrypoint in the private-file integration test so Vitest module transformation cannot mask this failure class again.tsxentrypoint used by the rebuild workflows.docker statsordocker system dfwhile BuildKit is active; host memory, pressure, top-RSS, load, and filesystem evidence remain.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration --project e2e-support test/e2e-private-file.test.ts test/e2e-risk-signal-reporter.test.ts test/pr-risk-plan.test.ts test/pr-e2e-gate-lifecycle.test.ts test/e2e/support/live-test-outcome-invocation.test.ts test/e2e/support/runner-pressure.test.ts(6 files, 157 tests passed);npx tsx tools/e2e/live-test-outcome.mtspassed;env E2E_PHASE=verify npx tsx tools/e2e/runner-pressure.mts snapshotpassed.; after the heartbeat fix,npx vitest run --project e2e-supportpassed 131 files and 1,260 tests with 2 skipped,npm run build:cliandnpm --prefix nemoclaw run buildpassed, and the normal pre-push CLI type-check 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: San Dang sdang@nvidia.com
Summary by CodeRabbit
0o600permissions, including max-size enforcement for read/write/append..mtsmodule entrypoint.live-test-outcomeexits successfully.