-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(onboard): preserve concurrent instance gateway and dashboard during onboard #4598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
6737a62
fix(onboard): refuse gateway recreate when live sandboxes exist
laitingsheng ab5648a
fixup: address review (extract live-row helper, wire-up test, docs, c…
laitingsheng 84bdda8
fix(onboard): narrow refuse-recreate to confirmed stale drift only
laitingsheng 429eb9f
Merge branch 'main' into fix/4422-refuse-gateway-drift-on-live-sandbox
laitingsheng 0ef1f56
refactor(state): introduce getGatewayName resolver for parallel-gatew…
laitingsheng a1c55fe
feat(registry): track per-sandbox gateway name with singleton backfill
laitingsheng fa45507
refactor(state): single source for gateway name + tighten registry ac…
laitingsheng fdddf53
refactor(state): replace remaining hard-coded gateway names with DEFA…
laitingsheng 48e58ae
refactor(state): persist + validate gatewayName at registry boundary
laitingsheng b9e28ba
fix(state): inline gatewayName validation to drop runner dep on platform
laitingsheng 7cf4f48
refactor(state): default getSandboxGatewayName + migrate reused entri…
laitingsheng 8b085b1
refactor(state): return null for unknown and corrupt gatewayName look…
laitingsheng ca876d6
refactor(state): route accessor diagnostics to stderr and cover reuse…
laitingsheng 6bb247f
test(state): load registry after HOME mutation and ensure regfile par…
laitingsheng 8fc38b0
refactor(state): relocate gateway-name accessor and route remaining s…
laitingsheng 2d1292f
refactor(state): route gateway-state recovery hints and regex through…
laitingsheng 1978b74
fix(state): inject active gateway name into reuse migration instead o…
laitingsheng 196725f
fix(state): drop unused DEFAULT_GATEWAY_NAME import from registry
laitingsheng 13d43ab
merge: resolve main into fix/4422-refuse-gateway-drift-on-live-sandbox
laitingsheng 79ca81b
Merge branch 'main' into fix/4422-refuse-gateway-drift-on-live-sandbox
cv 897e8aa
merge: resolve origin/main into branch by adopting main state
laitingsheng 40042b8
test(e2e): add concurrent-gateway-ports E2E and expand advisor catalog
laitingsheng 404de0c
fix(e2e): add missing fake OpenAI server heredoc + readiness poll
laitingsheng ac1c1a9
fix(ci): install NemoClaw before concurrent-gateway-ports E2E
laitingsheng 00a243e
fix(e2e): use custom provider + clear default sandbox before stages
laitingsheng 88a8385
fix(e2e): parse dashboard URL across lines + read phase from openshel…
laitingsheng f502f98
fix(onboard): skip gateway retire when foreign-active per-port gatewa…
laitingsheng 9de4b99
fix(onboard): preserve foreign sandbox's dashboard forward during pre…
laitingsheng 3ce01f0
fix(e2e): retry verify_sandbox_alive on Provisioning until Ready or t…
laitingsheng 939a182
fix(e2e): query each sandbox via its own gateway in verify_sandbox_alive
laitingsheng beb9014
Merge branch 'main' into fix/4422-refuse-gateway-drift-on-live-sandbox
laitingsheng 69dcdef
refactor(onboard): extract orphaned dashboard forward cleanup helper
laitingsheng c1e28bd
Merge remote-tracking branch 'origin/main' into fix/4422-refuse-gatew…
cv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { | ||
| tryCleanupOrphanedDashboardForward, | ||
| type OrphanedDashboardForwardDeps, | ||
| } from "../../../dist/lib/onboard/orphaned-dashboard-forward"; | ||
|
|
||
| function forwardListWith( | ||
| entries: Array<{ sandbox: string; port: number; status?: string }>, | ||
| ): string { | ||
| const header = "SANDBOX BIND PORT PID STATUS"; | ||
| const rows = entries.map( | ||
| (e) => `${e.sandbox} 127.0.0.1 ${e.port} 1234 ${e.status ?? "running"}`, | ||
| ); | ||
| return [header, ...rows].join("\n"); | ||
| } | ||
|
|
||
| interface MakeDepsOverrides { | ||
| cmdline?: string; | ||
| listFn?: () => string; | ||
| portCheckResult?: { ok: boolean; process?: string; pid?: number | null; reason?: string }; | ||
| } | ||
|
|
||
| function makeDeps(overrides: MakeDepsOverrides = {}) { | ||
| const calls = { | ||
| captureProcessArgs: vi.fn((_pid: number) => overrides.cmdline ?? "ssh -L openshell-forward 18789:..."), | ||
| runCaptureOpenshell: vi.fn( | ||
| overrides.listFn ?? (() => forwardListWith([])), | ||
| ) as OrphanedDashboardForwardDeps["runCaptureOpenshell"], | ||
| run: vi.fn() as unknown as OrphanedDashboardForwardDeps["run"], | ||
| sleepSeconds: vi.fn() as OrphanedDashboardForwardDeps["sleepSeconds"], | ||
| checkPortAvailable: vi.fn(async () => overrides.portCheckResult ?? { ok: true }) as unknown as OrphanedDashboardForwardDeps["checkPortAvailable"], | ||
| log: vi.fn(), | ||
| }; | ||
| const deps: OrphanedDashboardForwardDeps = { | ||
| port: 18789, | ||
| pid: 4321, | ||
| label: "Test dashboard", | ||
| captureProcessArgs: calls.captureProcessArgs, | ||
| runCaptureOpenshell: calls.runCaptureOpenshell, | ||
| run: calls.run, | ||
| sleepSeconds: calls.sleepSeconds, | ||
| checkPortAvailable: calls.checkPortAvailable, | ||
| log: calls.log, | ||
| }; | ||
| return { deps, calls }; | ||
| } | ||
|
|
||
| describe("tryCleanupOrphanedDashboardForward", () => { | ||
| it("returns not-openshell when the listener is unrelated SSH", async () => { | ||
| const { deps, calls } = makeDeps({ cmdline: "ssh -L 18789:remote-host:80 user@bastion" }); | ||
| const outcome = await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(outcome).toEqual({ kind: "not-openshell" }); | ||
| expect(calls.runCaptureOpenshell).not.toHaveBeenCalled(); | ||
| expect(calls.run).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns list-failed and skips the kill when forward list throws", async () => { | ||
| const { deps, calls } = makeDeps({ | ||
| listFn: () => { | ||
| throw new Error("gateway probe timed out"); | ||
| }, | ||
| }); | ||
| const outcome = await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(outcome).toEqual({ kind: "list-failed" }); | ||
| expect(calls.run).not.toHaveBeenCalled(); | ||
| expect(calls.checkPortAvailable).not.toHaveBeenCalled(); | ||
| expect(calls.log).toHaveBeenCalledWith( | ||
| expect.stringContaining("Could not enumerate OpenShell forwards"), | ||
| ); | ||
| }); | ||
|
|
||
| it("does not pass ignoreError to runCaptureOpenshell (failures must throw to be classified list-failed)", async () => { | ||
| const { deps, calls } = makeDeps(); | ||
| await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(calls.runCaptureOpenshell).toHaveBeenCalledWith( | ||
| ["forward", "list"], | ||
| expect.objectContaining({ timeout: 10_000, suppressOutput: true }), | ||
| ); | ||
| expect(calls.runCaptureOpenshell).not.toHaveBeenCalledWith( | ||
| ["forward", "list"], | ||
| expect.objectContaining({ ignoreError: true }), | ||
| ); | ||
| }); | ||
|
|
||
| it("returns owned-by-live when another live sandbox owns the port", async () => { | ||
| const { deps, calls } = makeDeps({ | ||
| listFn: () => forwardListWith([{ sandbox: "other-sandbox", port: 18789 }]), | ||
| }); | ||
| const outcome = await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(outcome).toEqual({ kind: "owned-by-live", owner: "other-sandbox" }); | ||
| expect(calls.run).not.toHaveBeenCalled(); | ||
| expect(calls.checkPortAvailable).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns killed-cleared when the kill frees the port", async () => { | ||
| const { deps, calls } = makeDeps({ portCheckResult: { ok: true } }); | ||
| const outcome = await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(outcome).toEqual({ kind: "killed-cleared" }); | ||
| expect(calls.run).toHaveBeenCalledWith(["kill", "4321"], { ignoreError: true }); | ||
| expect(calls.sleepSeconds).toHaveBeenCalledWith(1); | ||
| expect(calls.checkPortAvailable).toHaveBeenCalledWith(18789, undefined); | ||
| }); | ||
|
|
||
| it("returns killed-still-blocked when the kill ran but the port stayed blocked", async () => { | ||
| const refreshedCheck = { ok: false, process: "ssh", pid: 4321, reason: "still busy" }; | ||
| const { deps, calls } = makeDeps({ portCheckResult: refreshedCheck }); | ||
| const outcome = await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(outcome).toEqual({ kind: "killed-still-blocked", portCheck: refreshedCheck }); | ||
| expect(calls.run).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it("ignores non-live forward statuses when deciding ownership", async () => { | ||
| const { deps, calls } = makeDeps({ | ||
| listFn: () => forwardListWith([{ sandbox: "other-sandbox", port: 18789, status: "stopped" }]), | ||
| }); | ||
| const outcome = await tryCleanupOrphanedDashboardForward(deps); | ||
| expect(outcome.kind).toBe("killed-cleared"); | ||
| expect(calls.run).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.