feat(cli): converge readiness checks - #8738
Merged
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR converges host, gateway, onboarding, and rebuild flows on system-readiness reports. It adds read-only gateway probing, readiness admission, persisted gateway-authority checkpoints, stricter rebuild validation, and updated advisory and documentation contracts. ChangesSystem readiness and onboarding
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 89af2e0 in the TypeScript / code-coverage/cliThe overall coverage in commit 89af2e0 in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-8738.docs.buildwithfern.com/nemoclaw |
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / low confidence Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard/gateway-host-runtime-https.test.ts (1)
82-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore
OPENSHELL_GATEWAY_AUTH_TOKENafter this test.Line 83 directly mutates
process.env.OPENSHELL_GATEWAY_AUTH_TOKEN. The test restores onlyOPENSHELL_LOCAL_TLS_DIR. A later test can inherit this token.Capture and restore the original token, or use a test-scoped environment stub.
🤖 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 `@src/lib/onboard/gateway-host-runtime-https.test.ts` around lines 82 - 101, Restore the environment mutation in the gateway attachment test by capturing the original OPENSHELL_GATEWAY_AUTH_TOKEN before assigning it and restoring that value during cleanup, alongside OPENSHELL_LOCAL_TLS_DIR. Keep the existing probe assertions unchanged and ensure the token cannot leak into later tests.Source: Coding guidelines
🧹 Nitpick comments (10)
src/lib/readiness/gateway-production.ts (2)
173-182: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRead the gateway binary override from the probe environment.
resolveTrustedGatewayBinaryreadsprocess.env.NEMOCLAW_OPENSHELL_GATEWAY_BINdirectly. Every other resolution in this module takes the sanitizedprobeEnvthatbuildGatewayReadinessProbeEnvproduced, includingresolveTrustedOpenshellBinary. Accept the environment as a parameter so one environment governs the whole readiness probe and tests can control the override without mutatingprocess.env.🤖 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 `@src/lib/readiness/gateway-production.ts` around lines 173 - 182, Update resolveTrustedGatewayBinary to accept the sanitized probeEnv environment and read NEMOCLAW_OPENSHELL_GATEWAY_BIN from it instead of process.env. Pass the same probeEnv produced by buildGatewayReadinessProbeEnv through the readiness probe call chain, matching resolveTrustedOpenshellBinary, so tests can control the override without mutating process.env.
342-366: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the OpenShell status outputs instead of probing three more subprocesses.
observeReuseStaterunsopenshell status,openshell gateway info -g <name>, andopenshell gateway info.inspectLegacyClusterruns the same three commands again in the sameobserveManagedGatewaypass, and each command carries the fullOPENSHELL_PROBE_TIMEOUT_MSbudget. On a slow or wedged OpenShell CLI the readiness probe pays that cost twice. Pass the already captured outputs fromobserveReuseStateintoinspectLegacyCluster, and keep only the Docker inspections in the legacy path.🤖 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 `@src/lib/readiness/gateway-production.ts` around lines 342 - 366, Update observeManagedGateway and inspectLegacyCluster to accept and reuse the openshell status, named gateway info, and active gateway info captured by observeReuseState. Remove the duplicate captureReadonly subprocess calls from inspectLegacyCluster, while preserving its existing health and endpoint-binding checks; retain only the Docker inspections in the legacy path.src/lib/onboard/gateway-authority-checkpoint.test.ts (1)
104-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the unbound-authority rejection.
adoptPackagedGatewayAuthorityAfterTrustedInstallthrows "Trusted gateway installation requires a previously bound gateway authority." when the checkpoint decision is not selected. That fail-closed branch has no test. Add a case that calls the function on a fresh session without a priorbindGatewayAuthorityToCheckpointcall, and assert both the thrown message and thatsession.checkpointstays unchanged.🤖 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 `@src/lib/onboard/gateway-authority-checkpoint.test.ts` around lines 104 - 127, Add a test alongside the existing trusted-install cases that creates a fresh session without calling bindGatewayAuthorityToCheckpoint, snapshots session.checkpoint, and invokes adoptPackagedGatewayAuthorityAfterTrustedInstall. Assert it throws the message “Trusted gateway installation requires a previously bound gateway authority.” and that the checkpoint remains equal to the snapshot.src/lib/adapters/openshell/gateway-drift.ts (1)
433-442: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the installed-version dependency semantics across the three observers.
Line 258 and line 440 now call the injected getter whenever it exists, so an injected
nullstaysnull. Line 434 still usesdeps.getInstalledOpenshellVersion?.() ?? getInstalledOpenshellVersionOrNull(...), so an injected getter that returnsnullsilently falls back to the production probe. A caller that injects all three getters therefore gets probe execution ingetGatewayHostProcessDriftonly. Make the resolution rule identical in the three functions.♻️ Proposed alignment
const expectedVersion = - deps.getInstalledOpenshellVersion?.() ?? - getInstalledOpenshellVersionOrNull({ timeout: timeoutMs }); + typeof deps.getInstalledOpenshellVersion === "function" + ? deps.getInstalledOpenshellVersion() + : getInstalledOpenshellVersionOrNull({ timeout: timeoutMs });🤖 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 `@src/lib/adapters/openshell/gateway-drift.ts` around lines 433 - 442, Update the installed-version resolution in getGatewayHostProcessDrift to match the other observers: when deps.getInstalledOpenshellVersion exists, use its result directly, including null, and only call getInstalledOpenshellVersionOrNull when the getter is absent. Preserve the existing expectedVersion check and runtime resolution behavior.src/lib/readiness/onboard-admission.ts (1)
268-299: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared blocking-finding classification loop.
Lines 275-293 repeat the loop at Lines 256-261. Both iterate findings, skip non-blocking entries, call
canWaiveFinding, and split the IDs intowaivedFindingIdsandfindingIds. The only difference is the options value.A shared helper keeps one implementation of the waiver semantics, so a future waiver change cannot apply to one entry point and miss the other.
♻️ Proposed refactor
+function classifyBlockingFindings( + report: OnboardReadinessInput, + capabilities: ReadonlyMap<string, ReadinessCapability>, + options: Readonly<OnboardReadinessAdmissionOptions>, + managedGateway: boolean, +): { findingIds: string[]; waivedFindingIds: string[] } { + const findingIds: string[] = []; + const waivedFindingIds: string[] = []; + for (const finding of report.findings) { + if (!isBlocking(finding)) continue; + if (canWaiveFinding(finding, capabilities, options, managedGateway)) { + waivedFindingIds.push(finding.id); + } else findingIds.push(finding.id); + } + return { findingIds, waivedFindingIds }; +} + +const GATEWAY_CHECKPOINT_OPTIONS: OnboardReadinessAdmissionOptions = { + explicitlyOptedOutGpuPassthrough: false, + allowUnsupportedRuntime: false, + allowStorageRemediation: false, +};export function evaluateOnboardGatewayReadinessAdmission( report: OnboardReadinessInput, ): OnboardReadinessAdmissionDecision { const capabilities = new Map(report.capabilities.map((entry) => [entry.id, entry])); - const managedGateway = hasManagedGateway(report); - const waivedFindingIds: string[] = []; - const findingIds: string[] = []; - for (const finding of report.findings) { - if (!isBlocking(finding)) continue; - if ( - canWaiveFinding( - finding, - capabilities, - { - explicitlyOptedOutGpuPassthrough: false, - allowUnsupportedRuntime: false, - allowStorageRemediation: false, - }, - managedGateway, - ) - ) { - waivedFindingIds.push(finding.id); - } else { - findingIds.push(finding.id); - } - } + const { findingIds, waivedFindingIds } = classifyBlockingFindings( + report, + capabilities, + GATEWAY_CHECKPOINT_OPTIONS, + hasManagedGateway(report), + ); const capabilityIds: string[] = []; for (const id of GATEWAY_REQUIRED_CAPABILITIES) { addUnknownCapability(capabilityIds, capabilities, id); } return decisionFor(findingIds, capabilityIds, waivedFindingIds); }Apply the same helper inside
evaluateOnboardReadinessAdmission.🤖 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 `@src/lib/readiness/onboard-admission.ts` around lines 268 - 299, Extract the repeated blocking-finding classification logic into a shared helper that iterates findings, skips non-blocking entries, calls canWaiveFinding with supplied options, and returns waivedFindingIds and findingIds. Update both evaluateOnboardReadinessAdmission and evaluateOnboardGatewayReadinessAdmission to use this helper, passing their respective waiver options while preserving the existing managedGateway behavior.src/lib/readiness/host.ts (1)
103-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConvert
adaptHostAssessmentto an options object.The function now takes eight positional parameters. Six are optional and adjacent:
hostGpuPlatform,nvidiaGpuCount,nvidiaDriverVersion,platformIdentity,wslDockerDesktopGpuProofPassed, plus the newhasNvidiaGpu. The single call site at Lines 196-214 passes nested ternaries positionally, so a future insertion or reorder is easy to get wrong and hard to catch in review.A named-field argument removes the ordering dependency and makes the call site readable.
♻️ Proposed refactor sketch
+interface AdaptHostAssessmentInput { + architecture: string; + hasNvidiaGpu: boolean; + hostGpuPlatform?: NvidiaPlatform; + nvidiaGpuCount?: number; + nvidiaDriverVersion?: string; + platformIdentity?: PlatformIdentity; + wslDockerDesktopGpuProofPassed?: boolean; +} + function adaptHostAssessment( host: Readonly<HostAssessment>, - architecture: string, - hasNvidiaGpu: boolean, - hostGpuPlatform?: NvidiaPlatform, - nvidiaGpuCount?: number, - nvidiaDriverVersion?: string, - platformIdentity?: PlatformIdentity, - wslDockerDesktopGpuProofPassed?: boolean, + input: Readonly<AdaptHostAssessmentInput>, ): HostObservations {Also applies to: 196-214
🤖 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 `@src/lib/readiness/host.ts` around lines 103 - 112, Refactor adaptHostAssessment to accept a single options object containing host, architecture, hasNvidiaGpu, and the existing optional GPU/platform fields. Update its sole call site to pass named properties instead of positional arguments, preserving the current values and nested-ternary behavior.src/lib/onboard/runtime-control-flow.ts (1)
122-125: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the transition logic shared by the projection and the commit.
Lines 122-125 build
projectedSession, and lines 139-142 repeat the same two steps inside theupdateSessionmutator. Re-readingcurrentinside the mutator is correct, so the duplication is intentional in structure but not in logic.The risk is drift. If one copy gains a step,
plan.sessionno longer predicts whatcommit()persists, and a caller that rendersplan.sessionshows state that is never written. Extract one local function and call it from both places.♻️ Proposed extraction
+ const applyTransition = (candidate: Session): Session => { + const transitioned = resumeAgentChanged + ? deps.clearAgentScopedResumeState(candidate, selectedAgentName) + : candidate; + return updateSessionAgent(transitioned, input.selectedAgentName, deps); + }; const originalSession = structuredClone(input.session); - let projectedSession = structuredClone(input.session); - if (resumeAgentChanged) { - projectedSession = deps.clearAgentScopedResumeState(projectedSession, selectedAgentName); - } - projectedSession = updateSessionAgent(projectedSession, input.selectedAgentName, deps); + const projectedSession = applyTransition(structuredClone(input.session)); let committed: Promise<Session> | null = null;- return deps.updateSession((current) => { - const transitioned = resumeAgentChanged - ? deps.clearAgentScopedResumeState(current, selectedAgentName) - : current; - return updateSessionAgent(transitioned, input.selectedAgentName, deps); - }); + return deps.updateSession(applyTransition);Also applies to: 138-143
🤖 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 `@src/lib/onboard/runtime-control-flow.ts` around lines 122 - 125, Extract the shared transition steps from the projection and commit paths into one local helper, covering the resume-state clearing and updateSessionAgent operations. Use this helper to build projectedSession and inside the updateSession mutator, passing the mutator’s current session as its input so both plan.session and committed state apply identical logic.src/lib/onboard/fatal-runtime-preflight.ts (1)
336-394: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated refresh-admit-compose block.
Lines 342-355 and 384-394 are the same sequence: refresh the gateway projection, admit the gateway, recompute
managedGatewayReadiness, compose the composite report, and admit the composite report with an identical options object. The block also reassigns fourletbindings across about 50 lines.The behavior is correct today. The risk is drift: this is a security gate, and a future change applied to only one copy would silently weaken the other. Extract one helper and call it at both points.
♻️ Sketch of the extraction
+async function admitCompositeReadiness( + gateway: GatewayReadinessProjection, + result: FatalRuntimePreflightResult, + options: FatalRuntimePreflightOptions, + context: ReadinessGatedRuntimePreflightContext, + exitProcess: (code: number) => never, +): Promise<{ + gatewayReadiness: GatewayReadinessProjection; + managed: boolean; + readinessReport: SystemReadinessReport; +}> { + const gatewayReadiness = refreshGatewayReadinessProjection(gateway); + assertOnboardGatewayReadiness(gatewayReadiness, exitProcess); + const managed = isManagedGatewayReadiness(gatewayReadiness); + const readinessReport = composeSystemReadinessReport(result.readinessReport, gatewayReadiness); + assertOnboardSystemReadiness(readinessReport, result.host, { + explicitlyOptedOutGpuPassthrough: + result.sandboxGpuConfig.mode === "0" || options.optedOutGpuPassthrough === true, + resuming: context.resuming, + allowStorageRemediation: managed, + exitProcess, + }); + return { gatewayReadiness, managed, readinessReport }; +}As per coding guidelines: "Keep function complexity low, follow the Biome configuration".
🤖 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 `@src/lib/onboard/fatal-runtime-preflight.ts` around lines 336 - 394, Extract the repeated gateway and system readiness admission sequence into a helper near the existing onboarding readiness utilities, accepting the current gateway readiness, refreshed host result, options, context, and exitProcess dependencies and returning the updated gateway readiness, managedGatewayReadiness, and readinessReport values. Replace both post-refresh blocks in the surrounding preflight flow with calls to this helper, preserving the existing admission order and identical options, and keep the runtime GPU proof flow’s additional refresh between those calls unchanged.src/lib/onboard/openshell-install.ts (1)
292-292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument when
afterSuccessfulInstallruns.The callback fires after every version, messaging-feature, and maximum-version gate has passed, not immediately after
installOpenshell()returns. The option name alone does not convey that ordering, and the ordering matters for a reconciliation callback. Add a short doc comment on the option.♻️ Proposed refactor to document the callback timing
export function ensureOpenshellForOnboard( deps: OpenShellInstallDeps, - options: { afterSuccessfulInstall?(): void } = {}, + options: { + /** + * Runs after an install or upgrade succeeded and after every version and + * feature gate passed. Does not run when no install was needed. + */ + afterSuccessfulInstall?(): void; + } = {}, ): OpenShellInstallResult {🤖 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 `@src/lib/onboard/openshell-install.ts` at line 292, Add a concise documentation comment to the option defining afterSuccessfulInstall, stating that it runs only after installOpenshell completes and all version, messaging-feature, and maximum-version gates pass. Keep the callback invocation in the existing installed-success path unchanged.src/lib/onboard/machine/handlers/preflight.ts (1)
174-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the second
explicitlyOptedOutGpuPassthroughfrom the refreshed config.Line 175 computes
explicitlyOptedOutGpuPassthroughfromresumeSandboxGpuConfig.mode === "0". Line 196 hardcodesfalseafter line 188 recomputes the config from the freshdetectGpu()result. The two admissions therefore derive the same flag differently.The hardcoded value is correct while
mode === "0"follows only from an explicit disable flag or env override. IfresolveSandboxGpuConfiglater resolves the"auto"mode to"0"from a detection outcome, the second admission reports the wrong intent. Use the same expression in both places.The resolve, gateway-admission, and host-admission triplet also repeats in both passes. A small local helper would keep the two admissions aligned by construction.
♻️ Proposed refactor to align both admissions
await deps.assertGatewayReadiness(); const wslDockerDesktopGpuProofPassed = resolvedWslDockerDesktopGpuProof(gpu); deps.assertOnboardHostReadiness(resumeHost, gpu, { - explicitlyOptedOutGpuPassthrough: false, + explicitlyOptedOutGpuPassthrough: resumeSandboxGpuConfig.mode === "0", observedAt: hostObservedAt, now, ...(wslDockerDesktopGpuProofPassed === undefined ? {} : { wslDockerDesktopGpuProofPassed }), resuming: 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 `@src/lib/onboard/machine/handlers/preflight.ts` around lines 174 - 201, Update the second deps.assertOnboardHostReadiness call in the resume flow to derive explicitlyOptedOutGpuPassthrough from the refreshed resumeSandboxGpuConfig using the same expression as the first admission, rather than hardcoding false. Keep the refreshed resolve, gateway-readiness, and host-admission sequence aligned with the initial pass; a small local helper may encapsulate the repeated admission logic if needed.
🤖 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 `@scripts/install.sh`:
- Around line 3045-3050: Update the installer gate around assessHost and
planHostAdvisories to use the canonical readiness report and admission policy
for its exit status. Stop deriving blocking status from blockingActions or
remediation actions, and ensure unsupported runtimes are rejected consistently
with the readiness contract while preserving the existing successful path.
In `@src/lib/domain/docker-host.ts`:
- Around line 28-38: Update the Docker plain-text response parsing around
lowered and reachable to fail closed unless the text is a bare version token or
matches the Server Version format. Preserve acceptance of the existing “Podman
Engine” compatibility fixture, while treating permission and HTTP error messages
as unreachable. Add regression tests covering both error cases and the accepted
output forms.
In `@src/lib/onboard/docker-cdi.ts`:
- Around line 312-318: Update the docker CDI directory handling around
reportedDockerCdiSpecDirs and dockerCdiSpecDirs so the default-directory
fallback is used only when CDI support is confirmed. Preserve an unknown or
unsupported CDI state instead of treating an empty CDISpecDirs response as
supported, and ensure missing-spec findings do not recommend CDI generation for
daemons without confirmed CDI support.
In `@src/lib/onboard/docker-driver-gateway-service.ts`:
- Around line 637-641: Update gateway reuse to call the full trusted service
identity API instead of getTrustedActiveOpenShellGatewayUserServicePid, and
require identity.executablePath to be non-null before accepting a matching
listener PID. Remove the PID-only wrapper and its associated tests, preserving
executable validation for Homebrew and systemd paths.
In `@src/lib/onboard/fatal-runtime-preflight.test.ts`:
- Around line 253-256: Update the second assertOnboardHostReadiness test case to
store its exitProcess mock in a named variable, pass that mock into the call
with explicitlyOptedOutGpuPassthrough enabled, and assert the mock was not
called afterward.
In `@src/lib/onboard/gateway-host-runtime.ts`:
- Around line 183-208: Extract the shared trusted-install transition predicate
near sameGatewayOwner and use it in both adoption functions. In
src/lib/onboard/gateway-host-runtime.ts lines 183-208, replace the inline
transition checks in adoptPackagedGatewayOwnerAfterTrustedInstall; in
src/lib/onboard/gateway-authority-checkpoint.ts lines 74-107, replace the
corresponding checks in adoptPackagedGatewayAuthorityAfterTrustedInstall while
preserving the checkpoint-specific error text. Ensure the predicate captures the
complete permitted transition, including the checkpoint’s packaged-service
source assertion.
- Around line 199-203: Use the same “rerun onboarding” spelling in the Error
message constructed by the gateway authority validation flow, matching the
existing wording in the nearby onboarding message and leaving the rest of the
message unchanged.
In `@src/lib/onboard/machine/handlers/preflight.test.ts`:
- Around line 377-402: Retitle the test to describe that cached resume passes
the host observation timestamp to readiness and stops subsequent checks when
readiness rejects. In the assertOnboardHostReadiness mock, explicitly assert
that options.observedAt is present before parsing it, while preserving the
existing stale-age rejection and detectGpu/bridge non-invocation assertions.
In `@src/lib/onboard/preflight.ts`:
- Around line 758-766: Update the installer’s CDI repair import sources so
getNvidiaCdiSpecPath and isWslDockerDesktopRuntime resolve from their current
modules, or add compatibility re-exports from preflight.js. Ensure
scripts/install.sh can invoke CDI repair without its catch silently skipping the
operation.
In `@src/lib/onboard/runtime-control-flow.ts`:
- Around line 102-112: Add a test covering the public onboarding entrypoint that
invokes the branding flow, mock the onboarding layers as needed, and assert it
reaches planSelectedAgentTransition. Ensure the test verifies the public
boundary no longer depends on applySelectedAgentTransition and preserves the
branding call order after planning.
In `@src/lib/readiness/effects.test.ts`:
- Around line 138-139: Remove the unused startGateway and selectGateway spies
and any assertions against them from the createSystemReadinessReport test setup.
Keep the probeAttachment throwing stub and the observeManagedGateway assertions
that verify reachable gateway behavior.
In `@src/lib/readiness/gateway-production.test.ts`:
- Around line 331-335: Move the getTraceCollector() null assertion in the
probeAttachment test to immediately after await
deps.probeAttachment(externalOwner(gatewayPort)) and before
vi.stubEnv(TRACE_FILE_ENV, ""). Keep the subsequent environment reset and
fs.existsSync(tracePath) assertion unchanged.
In `@src/lib/readiness/gateway-production.ts`:
- Around line 577-585: In observePackagedServiceGatewayBinary, move the
executableAfter and generationAfter reads to after the
getTrustedActiveOpenShellGatewayUserServiceIdentity serviceAfter probe. Keep
executableBefore before the probe, so gatewayExecutableSamplesMatchTrustedBinary
compares samples that bracket the complete identity observation.
In `@src/lib/readiness/host.ts`:
- Around line 466-472: Update the capability logic around
host.gpu.container_toolkit_available to distinguish Jetson runtime applicability
from Docker evidence usability: return the Jetson runtime state when the check
is evaluable, return unknown when Jetson applies but dockerEvidenceUsable is
false, and retain the existing non-Jetson containerToolkitApplies behavior.
Apply the same evaluability guard to the related observation and
host.gpu.nvidia_runtime_missing finding so an unevaluable Jetson runtime reports
unknown without emitting an absent finding.
In `@src/lib/readiness/openshell-resolver.ts`:
- Line 4: Update the resolveOpenshell re-export in the readiness resolver module
to include the .js extension, and apply the same extension to the relative
re-export specifiers in the readiness system module. Preserve the existing
exports and paths otherwise.
In `@src/lib/readiness/platform-qualification.ts`:
- Around line 511-521: Align the severity of WSL findings with admission
behavior in the platform qualification flow: either mark reachable native Docker
and inconclusive WSL runtime findings as blocking, or explicitly exclude those
warning states from the host.platform.unsupported fallback so admission remains
advisory. Preserve other unsupported-platform blocking behavior and add coverage
for the selected path.
In `@src/lib/readiness/probe-env.ts`:
- Line 80: Validate controls.gatewayName before assigning it to
OPENSHELL_GATEWAY, using the same NUL, carriage-return, and line-feed rejection
already applied to localTlsDir. Preserve the existing conditional assignment
while returning the established clear readiness failure for invalid values
instead of passing them to the environment.
- Around line 112-118: Update the spawnSync invocation in the readiness probe
executor to provide a bounded default timeout and maxBuffer, while preserving
caller overrides by spreading spawnOptions after the defaults. Keep the existing
encoding, environment, shell, and stdio behavior unchanged.
In `@src/lib/readiness/system.test.ts`:
- Around line 170-178: Update the readiness test assertion around
report.capabilities to first require a non-empty array, then retain the existing
every-state check ensuring all capabilities are "unknown". Use the existing
report.capabilities collection so the stale-report behavior cannot pass when no
capabilities are emitted.
---
Outside diff comments:
In `@src/lib/onboard/gateway-host-runtime-https.test.ts`:
- Around line 82-101: Restore the environment mutation in the gateway attachment
test by capturing the original OPENSHELL_GATEWAY_AUTH_TOKEN before assigning it
and restoring that value during cleanup, alongside OPENSHELL_LOCAL_TLS_DIR. Keep
the existing probe assertions unchanged and ensure the token cannot leak into
later tests.
---
Nitpick comments:
In `@src/lib/adapters/openshell/gateway-drift.ts`:
- Around line 433-442: Update the installed-version resolution in
getGatewayHostProcessDrift to match the other observers: when
deps.getInstalledOpenshellVersion exists, use its result directly, including
null, and only call getInstalledOpenshellVersionOrNull when the getter is
absent. Preserve the existing expectedVersion check and runtime resolution
behavior.
In `@src/lib/onboard/fatal-runtime-preflight.ts`:
- Around line 336-394: Extract the repeated gateway and system readiness
admission sequence into a helper near the existing onboarding readiness
utilities, accepting the current gateway readiness, refreshed host result,
options, context, and exitProcess dependencies and returning the updated gateway
readiness, managedGatewayReadiness, and readinessReport values. Replace both
post-refresh blocks in the surrounding preflight flow with calls to this helper,
preserving the existing admission order and identical options, and keep the
runtime GPU proof flow’s additional refresh between those calls unchanged.
In `@src/lib/onboard/gateway-authority-checkpoint.test.ts`:
- Around line 104-127: Add a test alongside the existing trusted-install cases
that creates a fresh session without calling bindGatewayAuthorityToCheckpoint,
snapshots session.checkpoint, and invokes
adoptPackagedGatewayAuthorityAfterTrustedInstall. Assert it throws the message
“Trusted gateway installation requires a previously bound gateway authority.”
and that the checkpoint remains equal to the snapshot.
In `@src/lib/onboard/machine/handlers/preflight.ts`:
- Around line 174-201: Update the second deps.assertOnboardHostReadiness call in
the resume flow to derive explicitlyOptedOutGpuPassthrough from the refreshed
resumeSandboxGpuConfig using the same expression as the first admission, rather
than hardcoding false. Keep the refreshed resolve, gateway-readiness, and
host-admission sequence aligned with the initial pass; a small local helper may
encapsulate the repeated admission logic if needed.
In `@src/lib/onboard/openshell-install.ts`:
- Line 292: Add a concise documentation comment to the option defining
afterSuccessfulInstall, stating that it runs only after installOpenshell
completes and all version, messaging-feature, and maximum-version gates pass.
Keep the callback invocation in the existing installed-success path unchanged.
In `@src/lib/onboard/runtime-control-flow.ts`:
- Around line 122-125: Extract the shared transition steps from the projection
and commit paths into one local helper, covering the resume-state clearing and
updateSessionAgent operations. Use this helper to build projectedSession and
inside the updateSession mutator, passing the mutator’s current session as its
input so both plan.session and committed state apply identical logic.
In `@src/lib/readiness/gateway-production.ts`:
- Around line 173-182: Update resolveTrustedGatewayBinary to accept the
sanitized probeEnv environment and read NEMOCLAW_OPENSHELL_GATEWAY_BIN from it
instead of process.env. Pass the same probeEnv produced by
buildGatewayReadinessProbeEnv through the readiness probe call chain, matching
resolveTrustedOpenshellBinary, so tests can control the override without
mutating process.env.
- Around line 342-366: Update observeManagedGateway and inspectLegacyCluster to
accept and reuse the openshell status, named gateway info, and active gateway
info captured by observeReuseState. Remove the duplicate captureReadonly
subprocess calls from inspectLegacyCluster, while preserving its existing health
and endpoint-binding checks; retain only the Docker inspections in the legacy
path.
In `@src/lib/readiness/host.ts`:
- Around line 103-112: Refactor adaptHostAssessment to accept a single options
object containing host, architecture, hasNvidiaGpu, and the existing optional
GPU/platform fields. Update its sole call site to pass named properties instead
of positional arguments, preserving the current values and nested-ternary
behavior.
In `@src/lib/readiness/onboard-admission.ts`:
- Around line 268-299: Extract the repeated blocking-finding classification
logic into a shared helper that iterates findings, skips non-blocking entries,
calls canWaiveFinding with supplied options, and returns waivedFindingIds and
findingIds. Update both evaluateOnboardReadinessAdmission and
evaluateOnboardGatewayReadinessAdmission to use this helper, passing their
respective waiver options while preserving the existing managedGateway behavior.
🪄 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: 9f184c47-19e9-4fa0-ab8b-87aaad7cfba9
📒 Files selected for processing (105)
ci/platform-matrix.jsonci/source-architecture-budget.jsonci/source-shape-test-budget.jsonci/test-file-size-budget.jsondocs/about/how-it-works.mdxdocs/inference/choose-inference-provider.mdxdocs/reference/commands.mdxdocs/reference/platform-support.mdxdocs/reference/system-readiness.mdxscripts/install.shsrc/commands/host/probe.test.tssrc/commands/host/probe.tssrc/lib/actions/sandbox/gateway-target.tssrc/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-gpu-opt-out.tssrc/lib/actions/sandbox/rebuild-hermes-accepted-target.test.tssrc/lib/actions/sandbox/rebuild-onboard-dependencies.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-preflight-target-phase.test.tssrc/lib/actions/sandbox/rebuild-preflight-target-phase.tssrc/lib/actions/sandbox/rebuild-recreate-journal.test.tssrc/lib/actions/sandbox/rebuild-recreate-journal.tssrc/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/actions/sandbox/rebuild-recreate-phase.tssrc/lib/actions/sandbox/rebuild-recreate-reasoning.test.tssrc/lib/actions/sandbox/rebuild-resume-snapshot.test.tssrc/lib/actions/sandbox/rebuild-shields-finally.test.tssrc/lib/actions/sandbox/rebuild-target-runtime.tssrc/lib/adapters/openshell/gateway-drift.test.tssrc/lib/adapters/openshell/gateway-drift.tssrc/lib/advisories/checks/host/docker.test.tssrc/lib/advisories/checks/host/docker.tssrc/lib/advisories/checks/host/index.test.tssrc/lib/advisories/checks/host/runtime.test.tssrc/lib/advisories/checks/host/runtime.tssrc/lib/advisories/registry.test.tssrc/lib/advisories/registry.tssrc/lib/domain/docker-host.test.tssrc/lib/domain/docker-host.tssrc/lib/inference/nim.tssrc/lib/onboard.tssrc/lib/onboard/authoritative-rebuild-target.test.tssrc/lib/onboard/authoritative-rebuild-target.tssrc/lib/onboard/bridge-dns-preflight.tssrc/lib/onboard/docker-cdi.tssrc/lib/onboard/docker-driver-gateway-env.tssrc/lib/onboard/docker-driver-gateway-process-identity.test.tssrc/lib/onboard/docker-driver-gateway-process-identity.tssrc/lib/onboard/docker-driver-gateway-runtime.tssrc/lib/onboard/docker-driver-gateway-service-version-gate.test.tssrc/lib/onboard/docker-driver-gateway-service.test.tssrc/lib/onboard/docker-driver-gateway-service.tssrc/lib/onboard/fatal-runtime-preflight.test.tssrc/lib/onboard/fatal-runtime-preflight.tssrc/lib/onboard/gateway-authority-checkpoint.test.tssrc/lib/onboard/gateway-authority-checkpoint.tssrc/lib/onboard/gateway-host-runtime-https.test.tssrc/lib/onboard/gateway-host-runtime.test.tssrc/lib/onboard/gateway-host-runtime.tssrc/lib/onboard/gateway-http-readiness.tssrc/lib/onboard/machine/handlers/preflight.test.tssrc/lib/onboard/machine/handlers/preflight.tssrc/lib/onboard/machine/initial-flow-phases.test.tssrc/lib/onboard/machine/initial-flow-phases.tssrc/lib/onboard/openshell-cli.tssrc/lib/onboard/openshell-install.test.tssrc/lib/onboard/openshell-install.tssrc/lib/onboard/preflight-cdi.test.tssrc/lib/onboard/preflight-docker-host.test.tssrc/lib/onboard/preflight-messages.test.tssrc/lib/onboard/preflight-messages.tssrc/lib/onboard/preflight-readiness.test.tssrc/lib/onboard/preflight.test.tssrc/lib/onboard/preflight.tssrc/lib/onboard/remediation.tssrc/lib/onboard/runtime-control-flow.test.tssrc/lib/onboard/runtime-control-flow.tssrc/lib/onboard/runtime-provider/docker.test.tssrc/lib/onboard/runtime-provider/docker.tssrc/lib/onboard/sandbox-gpu-preflight.tssrc/lib/readiness/effects.test.tssrc/lib/readiness/gateway-production.test.tssrc/lib/readiness/gateway-production.tssrc/lib/readiness/gateway.test.tssrc/lib/readiness/gateway.tssrc/lib/readiness/host-production.test.tssrc/lib/readiness/host.test.tssrc/lib/readiness/host.tssrc/lib/readiness/index.tssrc/lib/readiness/onboard-admission.test.tssrc/lib/readiness/onboard-admission.tssrc/lib/readiness/openshell-resolver.tssrc/lib/readiness/platform-qualification.test.tssrc/lib/readiness/platform-qualification.tssrc/lib/readiness/presentation.test.tssrc/lib/readiness/presentation.tssrc/lib/readiness/probe-env.test.tssrc/lib/readiness/probe-env.tssrc/lib/readiness/sanitize.tssrc/lib/readiness/system.test.tssrc/lib/readiness/system.tstest/helpers/rebuild-flow-harness.tstest/helpers/rebuild-flow-test-harness.tstest/install-preflight.test.tstest/mcp-destroy-lifecycle.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.ts`:
- Around line 1405-1406: Remove the duplicate collectOnboardGatewayReadiness
declarations in src/lib/onboard.ts, retaining exactly one implementation that
delegates to preflightGatewayAuthority.collectOnboardGatewayReadiness with the
existing gatewayName, gatewayPort, resolveOwner, and probeAttachment arguments.
In `@src/lib/onboard/machine/preflight-gateway-authority.ts`:
- Around line 59-71: Apply the canonical gateway-readiness admission to the
refreshed gateway projection before computing externallySupervised or invoking
failFastOnForeignGatewayPortConflict in the preflight flow. Ensure blocking,
conflicting, or inconclusive refreshed states fail closed and prevent reuse and
lifecycle handling. Add coverage verifying a blocking refreshed projection stops
processing before those operations run.
🪄 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: a38ddd7d-b4e4-46b9-9628-a602039d1fd5
📒 Files selected for processing (9)
src/lib/actions/sandbox/rebuild-preflight-target-phase.test.tssrc/lib/onboard.tssrc/lib/onboard/docker-driver-gateway-service.test.tssrc/lib/onboard/fatal-runtime-preflight.test.tssrc/lib/onboard/machine/handlers/preflight.test.tssrc/lib/onboard/machine/initial-flow-phases.test.tssrc/lib/onboard/machine/preflight-gateway-authority.tssrc/lib/onboard/runtime-provider/docker.test.tssrc/lib/readiness/gateway-production.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/lib/actions/sandbox/rebuild-preflight-target-phase.test.ts
- src/lib/onboard/runtime-provider/docker.test.ts
- src/lib/onboard/machine/initial-flow-phases.test.ts
- src/lib/onboard/docker-driver-gateway-service.test.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/onboard/fatal-runtime-preflight.ts (1)
342-355: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated refresh-and-admit block.
Lines 342-355 and Lines 384-394 repeat the same five steps: refresh the gateway projection, assert gateway admission, recompute
managedGatewayReadiness, compose the system report, and assert system admission. One helper that takes the currentgatewayReadinessandrefreshedResultand returns the composed report plus the managed flag would remove the duplication and keep the two gates identical if the policy changes.Also applies to: 384-394
🤖 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 `@src/lib/onboard/fatal-runtime-preflight.ts` around lines 342 - 355, The refresh-and-admit sequence is duplicated in the onboarding flow. Extract a helper near the existing readiness logic that accepts the current gatewayReadiness and refreshedResult, performs refreshGatewayReadinessProjection, assertOnboardGatewayReadiness, managed readiness calculation, composeSystemReadinessReport, and assertOnboardSystemReadiness, then returns the composed report and managedGatewayReadiness; replace both blocks around the current and resumed paths with this helper while preserving their existing options and context.
🤖 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 `@src/lib/onboard/fatal-runtime-preflight.ts`:
- Around line 342-355: The refresh-and-admit sequence is duplicated in the
onboarding flow. Extract a helper near the existing readiness logic that accepts
the current gatewayReadiness and refreshedResult, performs
refreshGatewayReadinessProjection, assertOnboardGatewayReadiness, managed
readiness calculation, composeSystemReadinessReport, and
assertOnboardSystemReadiness, then returns the composed report and
managedGatewayReadiness; replace both blocks around the current and resumed
paths with this helper while preserving their existing options and context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fee0bb32-8dd8-4dac-b485-3534710f3d26
📒 Files selected for processing (106)
ci/platform-matrix.jsonci/source-architecture-budget.jsonci/source-shape-test-budget.jsonci/test-file-size-budget.jsondocs/about/how-it-works.mdxdocs/inference/choose-inference-provider.mdxdocs/reference/commands.mdxdocs/reference/platform-support.mdxdocs/reference/system-readiness.mdxscripts/install.shsrc/commands/host/probe.test.tssrc/commands/host/probe.tssrc/lib/actions/sandbox/gateway-target.tssrc/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-gpu-opt-out.tssrc/lib/actions/sandbox/rebuild-hermes-accepted-target.test.tssrc/lib/actions/sandbox/rebuild-onboard-dependencies.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-preflight-target-phase.test.tssrc/lib/actions/sandbox/rebuild-preflight-target-phase.tssrc/lib/actions/sandbox/rebuild-recreate-journal.test.tssrc/lib/actions/sandbox/rebuild-recreate-journal.tssrc/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/actions/sandbox/rebuild-recreate-phase.tssrc/lib/actions/sandbox/rebuild-recreate-reasoning.test.tssrc/lib/actions/sandbox/rebuild-resume-snapshot.test.tssrc/lib/actions/sandbox/rebuild-shields-finally.test.tssrc/lib/actions/sandbox/rebuild-target-runtime.tssrc/lib/adapters/openshell/gateway-drift.test.tssrc/lib/adapters/openshell/gateway-drift.tssrc/lib/advisories/checks/host/docker.test.tssrc/lib/advisories/checks/host/docker.tssrc/lib/advisories/checks/host/index.test.tssrc/lib/advisories/checks/host/runtime.test.tssrc/lib/advisories/checks/host/runtime.tssrc/lib/advisories/registry.test.tssrc/lib/advisories/registry.tssrc/lib/domain/docker-host.test.tssrc/lib/domain/docker-host.tssrc/lib/inference/nim.tssrc/lib/onboard.tssrc/lib/onboard/authoritative-rebuild-target.test.tssrc/lib/onboard/authoritative-rebuild-target.tssrc/lib/onboard/bridge-dns-preflight.tssrc/lib/onboard/docker-cdi.tssrc/lib/onboard/docker-driver-gateway-env.tssrc/lib/onboard/docker-driver-gateway-process-identity.test.tssrc/lib/onboard/docker-driver-gateway-process-identity.tssrc/lib/onboard/docker-driver-gateway-runtime.tssrc/lib/onboard/docker-driver-gateway-service-version-gate.test.tssrc/lib/onboard/docker-driver-gateway-service.test.tssrc/lib/onboard/docker-driver-gateway-service.tssrc/lib/onboard/fatal-runtime-preflight.test.tssrc/lib/onboard/fatal-runtime-preflight.tssrc/lib/onboard/gateway-authority-checkpoint.test.tssrc/lib/onboard/gateway-authority-checkpoint.tssrc/lib/onboard/gateway-host-runtime-https.test.tssrc/lib/onboard/gateway-host-runtime.test.tssrc/lib/onboard/gateway-host-runtime.tssrc/lib/onboard/gateway-http-readiness.tssrc/lib/onboard/machine/handlers/preflight.test.tssrc/lib/onboard/machine/handlers/preflight.tssrc/lib/onboard/machine/initial-flow-phases.test.tssrc/lib/onboard/machine/initial-flow-phases.tssrc/lib/onboard/machine/preflight-gateway-authority.tssrc/lib/onboard/openshell-cli.tssrc/lib/onboard/openshell-install.test.tssrc/lib/onboard/openshell-install.tssrc/lib/onboard/preflight-cdi.test.tssrc/lib/onboard/preflight-docker-host.test.tssrc/lib/onboard/preflight-messages.test.tssrc/lib/onboard/preflight-messages.tssrc/lib/onboard/preflight-readiness.test.tssrc/lib/onboard/preflight.test.tssrc/lib/onboard/preflight.tssrc/lib/onboard/remediation.tssrc/lib/onboard/runtime-control-flow.test.tssrc/lib/onboard/runtime-control-flow.tssrc/lib/onboard/runtime-provider/docker.test.tssrc/lib/onboard/runtime-provider/docker.tssrc/lib/onboard/sandbox-gpu-preflight.tssrc/lib/readiness/effects.test.tssrc/lib/readiness/gateway-production.test.tssrc/lib/readiness/gateway-production.tssrc/lib/readiness/gateway.test.tssrc/lib/readiness/gateway.tssrc/lib/readiness/host-production.test.tssrc/lib/readiness/host.test.tssrc/lib/readiness/host.tssrc/lib/readiness/index.tssrc/lib/readiness/onboard-admission.test.tssrc/lib/readiness/onboard-admission.tssrc/lib/readiness/openshell-resolver.tssrc/lib/readiness/platform-qualification.test.tssrc/lib/readiness/platform-qualification.tssrc/lib/readiness/presentation.test.tssrc/lib/readiness/presentation.tssrc/lib/readiness/probe-env.test.tssrc/lib/readiness/probe-env.tssrc/lib/readiness/sanitize.tssrc/lib/readiness/system.test.tssrc/lib/readiness/system.tstest/helpers/rebuild-flow-harness.tstest/helpers/rebuild-flow-test-harness.tstest/install-preflight.test.tstest/mcp-destroy-lifecycle.test.ts
🚧 Files skipped from review as they are similar to previous changes (99)
- src/lib/onboard/bridge-dns-preflight.ts
- src/lib/onboard/openshell-install.test.ts
- src/lib/advisories/checks/host/docker.test.ts
- scripts/install.sh
- ci/source-shape-test-budget.json
- docs/reference/commands.mdx
- src/lib/actions/sandbox/rebuild-shields-finally.test.ts
- src/lib/readiness/presentation.test.ts
- src/lib/advisories/checks/host/docker.ts
- src/lib/onboard/docker-driver-gateway-env.ts
- src/lib/actions/sandbox/gateway-target.ts
- src/lib/onboard/preflight-messages.ts
- src/lib/onboard/gateway-host-runtime.test.ts
- src/lib/advisories/checks/host/index.test.ts
- src/lib/readiness/probe-env.test.ts
- src/lib/onboard/preflight-messages.test.ts
- src/lib/onboard/remediation.ts
- src/lib/actions/sandbox/rebuild-gpu-opt-out.ts
- src/lib/onboard/docker-cdi.ts
- docs/about/how-it-works.mdx
- src/lib/actions/sandbox/rebuild-preflight-target-phase.test.ts
- src/commands/host/probe.ts
- src/lib/onboard/openshell-cli.ts
- src/lib/readiness/host-production.test.ts
- ci/test-file-size-budget.json
- src/lib/readiness/effects.test.ts
- src/lib/domain/docker-host.test.ts
- src/lib/actions/sandbox/rebuild-target-runtime.ts
- src/lib/onboard/machine/handlers/preflight.test.ts
- src/lib/actions/sandbox/rebuild-recreate-journal.test.ts
- src/lib/onboard/docker-driver-gateway-service.test.ts
- src/lib/actions/sandbox/rebuild-onboard-dependencies.ts
- src/lib/readiness/index.ts
- src/lib/actions/sandbox/rebuild-pipeline.ts
- src/lib/onboard/runtime-provider/docker.test.ts
- src/lib/actions/sandbox/rebuild-recreate-observability.test.ts
- src/lib/onboard/authoritative-rebuild-target.ts
- src/lib/onboard/runtime-control-flow.test.ts
- src/lib/actions/sandbox/rebuild-hermes-accepted-target.test.ts
- test/mcp-destroy-lifecycle.test.ts
- docs/inference/choose-inference-provider.mdx
- src/lib/onboard/docker-driver-gateway-service-version-gate.test.ts
- src/lib/onboard/preflight-docker-host.test.ts
- src/lib/onboard/machine/handlers/preflight.ts
- src/lib/actions/sandbox/rebuild-preflight-target-phase.ts
- src/lib/readiness/openshell-resolver.ts
- src/lib/onboard/runtime-control-flow.ts
- src/lib/advisories/checks/host/runtime.test.ts
- src/lib/onboard/preflight-cdi.test.ts
- src/lib/advisories/registry.test.ts
- src/lib/onboard/docker-driver-gateway-service.ts
- src/lib/actions/sandbox/rebuild-recreate-journal.ts
- src/lib/readiness/onboard-admission.ts
- test/install-preflight.test.ts
- src/lib/actions/sandbox/rebuild-recreate-reasoning.test.ts
- src/lib/readiness/sanitize.ts
- src/lib/onboard/gateway-host-runtime.ts
- src/lib/advisories/registry.ts
- src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts
- src/lib/readiness/gateway-production.test.ts
- src/lib/readiness/gateway.ts
- src/lib/onboard/gateway-authority-checkpoint.test.ts
- src/lib/onboard/machine/preflight-gateway-authority.ts
- src/lib/inference/nim.ts
- src/lib/readiness/platform-qualification.ts
- src/lib/actions/sandbox/rebuild-destroy-phase.test.ts
- src/commands/host/probe.test.ts
- src/lib/onboard/openshell-install.ts
- src/lib/onboard/docker-driver-gateway-runtime.ts
- src/lib/readiness/host.test.ts
- src/lib/onboard/docker-driver-gateway-process-identity.test.ts
- src/lib/readiness/platform-qualification.test.ts
- src/lib/onboard/authoritative-rebuild-target.test.ts
- src/lib/readiness/gateway.test.ts
- src/lib/onboard/gateway-authority-checkpoint.ts
- src/lib/onboard/machine/initial-flow-phases.ts
- src/lib/onboard/fatal-runtime-preflight.test.ts
- src/lib/onboard/runtime-provider/docker.ts
- src/lib/onboard/gateway-http-readiness.ts
- docs/reference/platform-support.mdx
- src/lib/advisories/checks/host/runtime.ts
- src/lib/onboard/machine/initial-flow-phases.test.ts
- src/lib/onboard/preflight.test.ts
- src/lib/readiness/presentation.ts
- ci/source-architecture-budget.json
- src/lib/onboard/preflight.ts
- src/lib/adapters/openshell/gateway-drift.test.ts
- src/lib/actions/sandbox/rebuild-recreate-phase.ts
- src/lib/onboard/gateway-host-runtime-https.test.ts
- src/lib/onboard/preflight-readiness.test.ts
- src/lib/readiness/host.ts
- test/helpers/rebuild-flow-test-harness.ts
- src/lib/onboard.ts
- ci/platform-matrix.json
- src/lib/adapters/openshell/gateway-drift.ts
- src/lib/onboard/sandbox-gpu-preflight.ts
- test/helpers/rebuild-flow-harness.ts
- src/lib/readiness/system.ts
- src/lib/domain/docker-host.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
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/rebuild-credential-preflight.test.ts`:
- Around line 54-55: Update the gateway startup flow in the rebuild credential
preflight test to request an operating-system-assigned port by starting the
child with port 0. Read the assigned port from the readiness marker, then derive
gatewayName, registry data, and command mocks from that assigned port instead of
process.pid and gatewayPortSequence.
🪄 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: 7bb6119c-08bd-49f8-9221-fd5d7b0a727b
📒 Files selected for processing (1)
test/rebuild-credential-preflight.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/onboard/machine/preflight-gateway-authority.ts (1)
39-43: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTie
gatewayPorttoGatewayOwner.The current caller passes the same value to both parameters, but the contract permits divergence. Derive the port from the resolved owner to make this invariant explicit, and test that the conflict check uses
owner.gatewayPort.🤖 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 `@src/lib/onboard/machine/preflight-gateway-authority.ts` around lines 39 - 43, Update EarlyGatewayPortConflictDeps and its conflict-check flow to derive the port from resolveOwner()’s returned GatewayOwner instead of accepting a separate gatewayPort value. Use owner.gatewayPort when invoking the conflict check, remove the redundant dependency field, and add coverage verifying the owner’s port is used.
🤖 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/machine/preflight-gateway-authority.ts`:
- Around line 45-54: Remove the legacy early port fast-fail helper
failFastOnEarlyGatewayPortConflict and its duplicate invocation from
preparePreflightGatewayAuthority, allowing collectOnboardGatewayReadiness to
perform canonical admission and emit structured gateway findings. Do not retain
the readiness-path exit unless a bounded compatibility window, retirement issue,
and exit criteria are explicitly documented.
In `@test/gateway-state-reconcile-2276.test.ts`:
- Line 294: Update the gateway fixture setup around gatewayPort and the listener
binding to use an ephemeral port (0), then read the listener’s assigned port and
consistently pass that value to the registry, gateway fixtures, and Docker
inspection response instead of hard-coding 8080.
- Around line 290-299: Update the docker inspect stub in the inspect branch to
parse the requested container target, accept only the expected gateway
container, and reject unexpected targets. Also validate the requested format
before returning each mocked gateway value, preserving the existing responses
only for recognized target-and-format combinations.
---
Nitpick comments:
In `@src/lib/onboard/machine/preflight-gateway-authority.ts`:
- Around line 39-43: Update EarlyGatewayPortConflictDeps and its conflict-check
flow to derive the port from resolveOwner()’s returned GatewayOwner instead of
accepting a separate gatewayPort value. Use owner.gatewayPort when invoking the
conflict check, remove the redundant dependency field, and add coverage
verifying the owner’s port is used.
🪄 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: 6b4a3a98-7b02-42ff-973f-37463151c789
📒 Files selected for processing (6)
src/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/onboard.tssrc/lib/onboard/machine/preflight-gateway-authority.tstest/exit-code-user-error-surfaces.test.tstest/gateway-state-reconcile-2276.test.tstest/onboard-inference-reconciliation.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/rebuild-recreate-observability.test.ts
- src/lib/onboard.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
cv
approved these changes
Aug 10, 2026
wscurran
added
area: cli
7 tasks
This was referenced Aug 10, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
Converges host, gateway, onboarding, resume, and rebuild readiness on one canonical report and admission policy. Blocking findings now stop lifecycle effects before credentials, images, gateways, policies, or sandboxes change, while public diagnostics remain observation-only and credential-free.
Related Issue
Fixes #7411
Changes
host probeand onboarding.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/about/how-it-works.mdx,docs/get-started/windows-preparation.mdx,docs/inference/choose-inference-provider.mdx,docs/reference/commands.mdx,docs/reference/platform-support.mdx,docs/reference/system-readiness.mdxDGX 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 testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: aggregatenpm testwas attempted locally, but shared macOS parallel temp ownership and Oclif startup contention caused unrelated cross-suite failures; affected [Readiness 5/5] Converge onboarding, gateway diagnostics, and doctor on readiness checks #7411 lanes pass serially and CI will run the aggregate projects in their normal environment.npm run docsbuilds without warnings (doc changes only) — command/result: passed with 0 errors and 2 Fern warnings.Signed-off-by: Julie Yaunches jyaunches@nvidia.com