test(onboard): exercise registry HTTPS proxy tunnel - #8185
Conversation
|
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 managed image registry proxy test now uses HTTPS, passes an abort signal to ChangesManaged image registry proxy
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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. 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: Julie Yaunches <jyaunches@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 `@src/lib/onboard/managed-image-registry-fetch.test.ts`:
- Around line 41-47: Update the test around the CONNECT handler and
session.fetchImpl rejection: keep the tunnel socket open while awaiting the
fetch failure, assert that the caught error has name "AbortError" from the
default abort reason, and destroy the socket in a finally block to ensure
cleanup.
🪄 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: 2a70ba63-e17e-4f9f-9b8d-1115a12664c6
📒 Files selected for processing (1)
src/lib/onboard/managed-image-registry-fetch.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
The managed registry proxy test now exercises the production-like HTTPS proxy route used by the registry client. Undici 8.10.0 sends an HTTP target as a plain proxy request, so the previous synthetic HTTP target could never reach the test's CONNECT handler and timed out in coverage shard 8.
Changes
AbortErrorrejection andregistry.invalid:443authority.finallycleanup.Stacked landing
This test-only PR is temporarily based on #8175 so CI evaluates the MCP audit fix and this Undici test correction together without creating a circular red dependency. Its public diff remains one test file. Once this PR is approved and green, it should merge into #8175; #8175 can then be revalidated and merged to
mainwhen its refreshed exact head is approved and green.Auto-merge is intentionally disabled.
Type of Change
Quality Gates
405bdd6c09e60205003cb032b5a1f7f088c9799d; the signed synchronization commit is empty and preserves reviewed treec3ad47e26a5387ce62fe95116730cc2777456719. This is a loopback-only test change with no production, dependency, credential, or external-network behavior.Documentation Writer Review
no-docs-needed405bdd6c09e60205003cb032b5a1f7f088c9799d, the PR still changes only the loopback HTTPS CONNECT fixture. The signed synchronization commit is empty and has the same tree as reviewed parent21ed6d13a. No production or user-visible behavior changed; no explanatory comments or test titles changed; the writing review found no findings.git diff --checkpassed and the focused Vitest file passed 5/5. A docs build is not applicable.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubgit diff --checkpassed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Signed-off-by: Julie Yaunches jmyaunch@gmail.com