ci(bazel): gate Java services on the framework row when nv-boot-parent changes - #696
ci(bazel): gate Java services on the framework row when nv-boot-parent changes#696balajinvda wants to merge 7 commits into
Conversation
Imports ngc/apps/nvcf-ui at 2c6b0b711b46c964cd4b3a119c5dd467540efbfc into src/uis/nvcf-ui, grouped by artifact kind alongside src/clis/nvcf-cli rather than under control-plane services: the UI is a shipped user-facing artifact, not a control-plane service. 267 of the upstream's 271 tracked files come across. Excluded: - .gitlab-ci.yml, superseded by this repository's release path - node_modules/.vite/.../results.json, a committed vitest cache artifact - .vscode/, editor-local settings Rewrites bare uppercase SIS to describe the cluster API in prose instead. All five occurrences were comments or documentation, so nothing structural changed. The OSS snapshot tooling has historically dropped whole files containing those bare tokens, which would have silently removed backend/cmd/server/main.go from the public mirror and broken the build with no error pointing at the cause. Adds /src/uis/ to CODEOWNERS under nvcf-control-plane-dev, per review request. The subtree keeps the upstream MODULE.bazel for now and is listed in .bazelignore, so the root module does not try to load packages whose labels are still relative to the upstream module root. That is deliberately temporary and runs against the recent consolidation of every nested module into the root; merging the JS rules into the root MODULE.bazel and rewriting the labels needs a real build to validate and belongs in its own change. Not included, and tracked in #634: - root MODULE.bazel merge and the bazel.yml matrix row - an npm collector for tools/collect-dependencies; until it exists the UI's third-party tree does not reach dependencies.md NOTICE is regenerated to absorb the UI's own NOTICE. All 182 imported source files already carry SPDX headers, and tools/ci/check-license passes. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
Two CI failures from the import. check-nested-modules rejected src/uis/nvcf-ui because a new nested MODULE.bazel is unclassified. Recorded as `ledger`, not `exception`: the category means the migration is outstanding rather than excused, and the entry leaves the file entirely once the subtree builds from the root module. This does re-open the consolidation backlog, which had reached zero services awaiting migration. dependencies.md was stale because the UI's Go backend pulls in github.com/rs/xid. Regenerated with the real generator rather than hand-applying the diff CI printed, since the two have disagreed before. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
The import landed source-only: the subtree kept its upstream MODULE.bazel and sat in .bazelignore, so nothing in this repository built, tested or linted its 185 TypeScript and 29 Go files. Excluding the upstream .gitlab-ci.yml had also removed the six quality gates it used to run, replacing them with none. This finishes the migration so the code is actually verified here. check-nested-modules now reports `awaiting migration: 0` again, so the import no longer works against the consolidation. The merge, in the order the failures surfaced: - Lifted aspect_rules_js and rules_nodejs plus the node and npm extensions into the root MODULE.bazel, repointed at //src/uis/nvcf-ui/ui:pnpm-lock.yaml. - Registered the backend in go.work.bazel and rewrote subtree-relative labels. - Dropped the subtree's MODULE.bazel, .bazelrc, rules/ and platforms/; the root already provides distroless_static, rules/oci and every Go dependency the backend imports. - Lifted the subtree's .bazelignore entries into the root. aspect_rules_js refuses to evaluate npm_translate_lock unless every nested node_modules pnpm will create is ignored, so this is required, not tidiness. - Ported go_web_oci_image into the root rules/oci. The root copy was not equivalent; it had no such rule. - Dropped that rule's repo_tag parameter. Its own doc said it existed because package_name() is empty at a repo root; at src/uis/nvcf-ui it is not, so the root's derived load tag applies and the workaround is obsolete. - Removed //platforms:sources and //rules/oci:sources from the container's source bundle. Those are root build infrastructure, not UI source. - Aligned the backend's controller-runtime from v0.24.1 to v0.22.5 to match the rest of the repository. A version-specific replace pins client-go to v0.34.2 while MVS was selecting controller-runtime v0.24.1 from this module, and 0.24 needs client-go APIs that 0.34 does not have. Merges main for the pnpm dependency collector added in #657. Without it the UI's npm tree never reached dependencies.md; the regenerated file now records 531 Node.js packages. Adds the nvcf-ui matrix row so the subtree builds and tests on its own changes. Verified locally: `bazel build //src/uis/nvcf-ui/...` covers 31 targets and `bazel test //src/uis/nvcf-ui/...` passes 5 of 5, including the frontend vitest suite. The controller-runtime downgrade is proven at compile and unit-test level only. The token-watcher is the component using it, and its tests do not exercise cache behaviour, so a reviewer from the UI team should confirm 0.22.5 is acceptable at runtime. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
`bazel (root)` failed while `bazel (nvcf-ui)` passed. Adding the backend to go.work.bazel changes dependency resolution for every Go module in the repository, not just this subtree, and MVS took k8s.io/apiextensions-apiserver from v0.34.1 to v0.36.0 because this module still required k8s.io at v0.36.0. A version-specific replace pins client-go to v0.34.2, so the v0.36 apiextensions code referenced symbols that version does not have. Pins k8s.io/api, apimachinery and client-go to v0.34.2 and apiextensions-apiserver to v0.34.1, matching the rest of the repository. Applied with `go mod edit` and deliberately no `go mod tidy`: tidy re-resolves these upward from the transitive graph, which is what silently undid the equivalent fix for controller-runtime earlier. Verified with a full `bazel build //...` rather than the subtree alone. The subtree-only build is what let this through: it cannot observe a change in workspace-wide version resolution. Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
TestLoadConfigAuthCredentials failed in CI on this branch while passing on
main, and the difference was not the code. On main the test is a remote-cache
hit on every run and has never actually executed in CI. Changing
go.work.bazel here invalidated the cache key, so this is its first real run.
The test is not hermetic. state.DefaultStateManager is constructed at package
init, capturing $HOME before any test can change it, so the test's
t.Setenv("HOME", t.TempDir()) does not move the state path. LoadConfig falls
back to that state file for credentials, so a stray ~/.nvcf-cli.state supplies
an API key: "nothing set" then finds credentials, and "complete oauth2 setup"
resolves to bearer instead of oauth2. CI runs tests with HOME=/tmp, which is
shared and writable.
Adds state.ResetDefaultStateManager so a test can rebuild the manager after
pointing HOME somewhere private, and calls it at both sites that override HOME.
Production behaviour is unchanged: a CLI process's home directory does not
change after start, and nothing outside tests calls it.
Reproduced before fixing by seeding $HOME/.nvcf-cli.state with an apiKey and
running against main, which fails exactly as CI did; the same command passes
with this change. Full suite: 21 of 21 pass with --nocache_test_results.
Worth noting beyond this test: cache hits can mask tests that would fail if
they ran. A periodic uncached run on main would surface others like it.
Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
…t changes
Sanjay observed nv-boot-parent building alongside the services that depend on
it. Measuring three runs shows it is worse than concurrent: it reliably
finishes last.
services start +0s +5s +6s +80s nv-boot-parent +228s
+0s +0s +2s +33s +465s
+0s +6s +7s +7s +271s
Two things combine. The matrix is built from
`find src -name bazel-java-ci.json | LC_ALL=C sort`, which puts
src/libraries/java/nv-boot-parent after src/control-plane-services/*, so the
framework is the last of five rows. The docker-host lane caps at
max-parallel 4. The framework therefore cannot start until a service frees a
slot, and the services compile against whatever the cache already held.
The framework row now runs as its own job and the service rows depend on it.
The gate is conditional: the framework job only exists when change detection
selected it, which is exactly when nv-boot-parent changed. When it did not,
that job is skipped and the services start immediately, so ordinary runs pay
nothing. When it did, services wait the 2.4 to 5.1 minutes the framework takes
and then compile against the library they were meant to.
`always()` on the service job is required rather than stylistic: a skipped
dependency would otherwise skip every service, which is the common case.
To avoid duplicating the job, its 131 step lines move to a composite action at
.github/actions/bazel-java-build. Both jobs are thin callers, so the workflow
is 62 lines added against 128 removed. The steps were extracted mechanically,
not retyped; matrix.subtree.* became inputs.*, and every run: step gained the
explicit shell: composite actions require.
The required-checks aggregator now evaluates the framework result. Without it a
framework failure would skip the service rows and the aggregate would pass on
their skipped state.
actionlint is clean. Not yet exercised on a branch that actually changes
nv-boot-parent, which is the run that proves the gate fires; that is the next
step, not something this commit claims.
Co-authored-by: Balaji Ganesan <bganesan@nvidia.com>
📝 WalkthroughWalkthroughChangesThe pull request adds the NVCF UI frontend and Go backend, including API specifications, mock data, routing, dashboards, cluster and function views, health monitoring, token watching, Helm deployment resources, Bazel packaging, and CI workflows. NVCF UI platform — Root Bazel, OCI, Go workspace, dependency, ownership, documentation, licensing, and CI integration were added. Backend — HTTP proxying, static SPA serving, Kubernetes health checks, control-plane status reporting, heartbeat liveness, token rotation, middleware, and utility packages were added. Frontend — React routing, account switching, dashboard, clusters, functions, reusable data tables, generated API configuration, MSW scenarios, shared hooks, and test infrastructure were added. Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
|
Closing: this branch was cut inside the nvcf-ui worktree, so it carried the whole UI import (268 files). Reopened clean as a 2-file change. |
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (28)
src/clis/nvcf-cli/internal/client/client_test.go-1213-1215 (1)
1213-1215: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore the default state manager after each test.
state.ResetDefaultStateManager()replaces the package-globalstate.DefaultStateManager. These tests do not restore the previous pointer. Aftert.TempDir()cleanup, later tests can use a manager whose state path points to a deleted directory. This makes the test suite order-dependent.Save the previous manager before changing
HOMEand restore it witht.Cleanup()at both call sites. Follow the existing pattern insrc/clis/nvcf-cli/cmd/root_test.go.Proposed fix
+ previousDefaultManager := state.DefaultStateManager + t.Cleanup(func() { + state.DefaultStateManager = previousDefaultManager + }) t.Setenv("HOME", t.TempDir()) state.ResetDefaultStateManager()Also applies to: 1312-1314
🤖 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/clis/nvcf-cli/internal/client/client_test.go` around lines 1213 - 1215, At both call sites around state.ResetDefaultStateManager in the client tests, save the current state.DefaultStateManager before changing HOME, then register t.Cleanup to restore that pointer after each test. Preserve the existing reset behavior while ensuring later tests cannot retain a manager referencing the temporary directory.src/uis/nvcf-ui/ui/src/main.tsx-42-60 (1)
42-60: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle bootstrap failures instead of rendering a blank page.
Two failure paths produce a blank page with no user-visible message and no logged error:
enableMocking()has no.catch(). Ifworker.start()rejects, for example when the service worker fails to register, the.then()callback never runs and React never mounts. The only signal is an unhandled promise rejection.- If
document.getElementById("app")returnsnull, the guard silently skips rendering.Mount the app even when mock setup fails, and report a missing root element.
🛠️ Proposed fix
-enableMocking().then(() => { - const rootElement = document.getElementById("app"); - if (rootElement && !rootElement.innerHTML) { - const root = ReactDOM.createRoot(rootElement); - root.render( +function mount() { + const rootElement = document.getElementById("app"); + if (!rootElement) { + throw new Error('Root element "`#app`" not found'); + } + if (rootElement.innerHTML) return; + const root = ReactDOM.createRoot(rootElement); + root.render( <ThemeProvider density="standard" global target="html" theme={getStoredTheme()} > <QueryClientProvider client={queryClient}> <RouterProvider context={{ queryClient }} router={router} /> <ReactQueryDevtools /> </QueryClientProvider> </ThemeProvider>, - ); - } -}); + ); +} + +enableMocking() + .catch((error) => { + console.error("Mock service worker failed to start", error); + }) + .finally(mount);Adjust the indentation of the JSX block when you apply the change.
🤖 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/uis/nvcf-ui/ui/src/main.tsx` around lines 42 - 60, Update the bootstrap chain around enableMocking() so a rejected mock setup is caught, logged, and still proceeds to mount the React application. In the root-element guard, explicitly report the missing app element instead of silently skipping rendering; preserve normal rendering when the element exists and adjust the JSX indentation to match the updated control flow.src/uis/nvcf-ui/ui/orval.config.ts-58-64 (1)
58-64: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSeparate the Orval project output directories.
Orval
8.9.0runsclean: truebefore generatingcontrol-planeand cleans both shared output paths. A targetedcontrol-planerun can therefore removenvcfandsisgenerated files. Use project-specifictargetandschemas.pathvalues, or clean once before full generation.🤖 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/uis/nvcf-ui/ui/orval.config.ts` around lines 58 - 64, Update the "control-plane" Orval configuration to avoid clean: true deleting generated files for the nvcf and sis projects: assign project-specific output target and schemas.path locations, or move cleaning to a single full-generation step. Preserve the existing shared output behavior while ensuring targeted control-plane generation cannot clean other project directories.src/uis/nvcf-ui/ui/src/features/functions/hooks/useFunctionsWithDeployments.ts-64-74 (1)
64-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the deployments query best-effort.
If the deployments request fails without cached data,
useSuspenseQueriesthrows and preventsFunctionsListfrom rendering. Handle that failure as an empty enrichment result or use a non-throwing query path. Add a test that rejects the deployments request and verifies that the functions list still renders.🤖 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/uis/nvcf-ui/ui/src/features/functions/hooks/useFunctionsWithDeployments.ts` around lines 64 - 74, Update useFunctionsWithDeployments so failures from getGetAllFunctionDeploymentsSuspenseQueryOptions do not cause useSuspenseQueries to throw when no cached deployments exist; handle the deployments result as an empty enrichment while preserving successful deployments. Add a test covering a rejected deployments request and verify that FunctionsList still renders.src/uis/nvcf-ui/ui/src/features/functions/FunctionDetail.tsx-208-215 (1)
208-215: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe invoke command hardcodes the
/echopath for every function.Line 210 builds the URL with a literal
/echopath. The page already reads the real endpoint intofn.inferenceUrland displays it at line 395. Every function therefore shows a copyable command that targets/echo, which does not match most functions. A user who copies the command receives a failing request.Build the path from
fn.inferenceUrl, and fall back to a documented placeholder when it is absent.🤖 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/uis/nvcf-ui/ui/src/features/functions/FunctionDetail.tsx` around lines 208 - 215, Update the invocationCommand URL construction in FunctionDetail to derive the request path from fn.inferenceUrl instead of hardcoding /echo. When fn.inferenceUrl is absent, use the documented placeholder, while preserving the existing gateway address interpolation and command formatting.src/uis/nvcf-ui/ui/src/features/accounts/getActiveNcaId.ts-25-27 (1)
25-27: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd
revalidateIfStale: trueto theensureQueryDatacall.This helper resolves the active account ID for route loaders. The call omits
revalidateIfStale: true. Without it,ensureQueryDatareturns a stale cached account list and does not refetch in the background. An account that was added or removed server-side stays missing until the query is invalidated.Proposed change
const response = await queryClient.ensureQueryData({ - getGetCloudAccountsQueryOptions(), - ); + ...getGetCloudAccountsQueryOptions(), + revalidateIfStale: true, + });Based on the coding guideline "Use
ensureQueryDatawithrevalidateIfStale: truein route loaders."🤖 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/uis/nvcf-ui/ui/src/features/accounts/getActiveNcaId.ts` around lines 25 - 27, Update the ensureQueryData call in the active account ID loader to pass revalidateIfStale: true alongside getGetCloudAccountsQueryOptions(), ensuring stale account data is refreshed while preserving the existing query and return behavior.Source: Coding guidelines
src/uis/nvcf-ui/ui/src/features/accounts/components/AccountSwitcher.tsx-48-48 (1)
48-48: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the character access before calling
toUpperCase().
activeAccount?.name?.[0]guards onlynamebeingnullorundefined. It does not guard the result of the index access. If the API returns an account with an emptyname,name?.[0]evaluates toundefinedand.toUpperCase()throws aTypeError.AccountSwitcherrenders in the root shell, so this crash affects every route.Add an optional chain on the index result.
Proposed fix
<Avatar - fallback={activeAccount?.name?.[0].toUpperCase()} + fallback={activeAccount?.name?.[0]?.toUpperCase()} interactive size="small" />🤖 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/uis/nvcf-ui/ui/src/features/accounts/components/AccountSwitcher.tsx` at line 48, Update the fallback expression in AccountSwitcher to optional-chain the character returned by activeAccount?.name?.[0] before calling toUpperCase(), so empty account names render safely without throwing.src/uis/nvcf-ui/ui/src/features/functions/FunctionDetail.tsx-403-409 (1)
403-409: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse
AsyncBoundaryfor this component-level suspense boundary.
InstanceTypesPanelis wrapped in a bareSuspense. The boundary handles the pending state but not the error state. If the deployment request fails, the error propagates to the route error boundary and replaces the whole function detail page. The rest of the page does not depend on that request. The repository guideline requiresAsyncBoundaryfor component-level, non-route suspense and error isolation.Proposed change
- <Suspense fallback={<InstanceTypesPanelSkeleton />}> + <AsyncBoundary pendingFallback={<InstanceTypesPanelSkeleton />}> <InstanceTypesPanel functionId={functionId} ncaId={ncaId} versionId={versionId} /> - </Suspense> + </AsyncBoundary>Match the prop names to the
AsyncBoundarysignature in~/components/AsyncBoundary.Based on the coding guideline "Use
AsyncBoundaryfor component-level, non-route suspense and error isolation."🤖 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/uis/nvcf-ui/ui/src/features/functions/FunctionDetail.tsx` around lines 403 - 409, Replace the bare Suspense wrapper around InstanceTypesPanel with AsyncBoundary, using the component’s existing InstanceTypesPanelSkeleton as the pending fallback and matching the error/fallback prop names defined by ~/components/AsyncBoundary. Keep functionId, ncaId, and versionId unchanged.Source: Coding guidelines
src/uis/nvcf-ui/ui/src/features/dashboard/routes.tsx-35-50 (1)
35-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle rejected cache-warming promises.
Each
ensureQueryDatacall can reject when its API request fails.voiddiscards the promise reference but does not consume the rejection. This can create unhandled-rejection events even though each panel is intended to show its own error state.Attach a rejection handler to each cache-warming promise, or aggregate them and explicitly discard failures after React Query records the query error.
🤖 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/uis/nvcf-ui/ui/src/features/dashboard/routes.tsx` around lines 35 - 50, Update the cache-warming calls in the route loader to consume rejected promises from each ensureQueryData invocation. Attach a rejection handler to every call, or aggregate the promises and explicitly handle failures, while preserving React Query’s recorded query errors for the panels.Source: Coding guidelines
src/uis/nvcf-ui/ui/src/styles.css-23-26 (1)
23-26: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReplace the legacy directive with a scoped Tailwind v4 utilities import.
Keep utilities scoped to
#style-rootby using Tailwind v4’s@layer utilitiespattern with@import "tailwindcss/utilities.css" layer(utilities);. Do not use the full@import "tailwindcss";import unless base and theme layers are also required. This UI uses Biome, not Stylelint; no Stylelint change is required for@source.🤖 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/uis/nvcf-ui/ui/src/styles.css` around lines 23 - 26, Replace the legacy `@tailwind` utilities directive inside `#style-root` with a scoped Tailwind v4 `@layer` utilities block importing tailwindcss/utilities.css into the utilities layer. Preserve the existing `@source` directive and do not add the full Tailwind import or any Stylelint configuration changes.Source: Linters/SAST tools
src/uis/nvcf-ui/backend/cmd/control-plane/BUILD.bazel-21-21 (1)
21-21: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe
importpathdoes not match the package location.The path segment is
src/control-plane-services/nvcf-ui/backend/cmd/control-plane, but the package lives atsrc/uis/nvcf-ui/backend/cmd/control-plane. The same mismatch exists in the siblingBUILD.bazelfiles and originates from thegazelle:prefixinsrc/uis/nvcf-ui/backend/BUILD.bazel.🤖 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/uis/nvcf-ui/backend/cmd/control-plane/BUILD.bazel` at line 21, The Bazel import path is using the wrong repository prefix. Update the gazelle:prefix in the nvcf-ui backend BUILD configuration from src/control-plane-services to src/uis, then regenerate or correct the sibling BUILD.bazel importpath entries so they match their actual package locations under src/uis/nvcf-ui.src/uis/nvcf-ui/backend/cmd/server/BUILD.bazel-21-21 (1)
21-21: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe
importpathdoes not match the package location.The path segment is
src/control-plane-services/nvcf-ui/backend/cmd/server, but the package lives atsrc/uis/nvcf-ui/backend/cmd/server. The root cause is thegazelle:prefixinsrc/uis/nvcf-ui/backend/BUILD.bazel.🤖 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/uis/nvcf-ui/backend/cmd/server/BUILD.bazel` at line 21, Update the gazelle:prefix configuration in the backend BUILD file so generated import paths use src/uis rather than src/control-plane-services, then regenerate or correct the server BUILD target’s importpath to match the package location under src/uis/nvcf-ui/backend/cmd/server.src/uis/nvcf-ui/backend/internal/control-plane/BUILD.bazel-27-27 (1)
27-27: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe
importpathdoes not match the package location.The path segment is
src/control-plane-services/nvcf-ui/backend/internal/control-plane, but the package lives atsrc/uis/nvcf-ui/backend/internal/control-plane. The root cause is thegazelle:prefixinsrc/uis/nvcf-ui/backend/BUILD.bazel.🤖 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/uis/nvcf-ui/backend/internal/control-plane/BUILD.bazel` at line 27, The BUILD target’s importpath uses the wrong repository directory prefix. Update the gazelle:prefix in the backend BUILD configuration so generated import paths use src/uis/nvcf-ui/backend, then regenerate or correct the importpath for the control-plane package accordingly..github/workflows/bazel.yml-923-926 (1)
923-926: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftDo not serialize PR service jobs behind a cache-read-only framework job.
pull_requestruns set--remote_upload_local_results=false, sonv-boot-parentcannot publish compiled outputs to Buildbarn. Service targets still depend on its targets and may rebuild them after the gate. Compare PR and main-push ormerge_groupwall time and cache-hit rate, then remove or redesign the PR gate if it adds no cache benefit.🤖 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 @.github/workflows/bazel.yml around lines 923 - 926, Remove or redesign the bazel-java-framework dependency from the bazel-docker job for pull_request runs so service jobs are not serialized behind a cache-read-only framework job. Preserve any necessary dependency constraints for main-push or merge_group workflows, and compare wall time and cache-hit results to confirm the gate provides measurable benefit before retaining it.src/uis/nvcf-ui/BUILD.bazel-67-73 (1)
67-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
strip_prefix.from_root("")keeps the full workspace path, so the bundle lands at the wrong location.
source_filesaggregates files from this package and from//src/uis/nvcf-ui/backend:sourcesand//src/uis/nvcf-ui/ui:sources. Their workspace-root paths all begin withsrc/uis/nvcf-ui/. Stripping an empty prefix preserves those paths, so the layer places the sources at/usr/share/nvcf-ui/src/src/uis/nvcf-ui/...instead of/usr/share/nvcf-ui/src/.... The comment at line 34 states the intended path. Abazel buildcannot detect this, because only the tarball is assembled.🐛 Proposed fix
pkg_tar( name = "source_layer", srcs = [":source_files"], package_dir = "/usr/share/nvcf-ui/src", - strip_prefix = strip_prefix.from_root(""), + strip_prefix = strip_prefix.from_root("src/uis/nvcf-ui"), visibility = ["//visibility:private"], )🤖 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/uis/nvcf-ui/BUILD.bazel` around lines 67 - 73, Update the source_layer pkg_tar rule’s strip_prefix configuration to remove the shared workspace prefix src/uis/nvcf-ui/ from source_files, so files from this package, backend:sources, and ui:sources are placed directly under /usr/share/nvcf-ui/src as specified by the existing comment near line 34..github/workflows/bazel.yml-152-153 (1)
152-153: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winFix the
source_layerlabel.
src/uis/nvcf-ui/BUILD.bazeldefines:source_layer, but the image passes//:source_layer, which resolves to the root package. No root target exists, so Bazel analysis fails. Usesource_layer = ":source_layer"or//src/uis/nvcf-ui:source_layer.🤖 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 @.github/workflows/bazel.yml around lines 152 - 153, Update the nvcf-ui image configuration entry in the workflow to reference the package-local source_layer target instead of the root //:source_layer label. Use the existing :source_layer target defined by src/uis/nvcf-ui/BUILD.bazel, preserving the other image configuration fields.src/uis/nvcf-ui/.claude/skills/codegen/SKILL.md-36-39 (1)
36-39: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAlign Zod generation with route-validation guidance.
The Orval configuration generates only TypeScript models, so
~/generated/model/getWidgetsParams.zoddoes not exist. Enable and commit the required Zod output, or replace the example with validation APIs produced by the configured generator.🤖 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/uis/nvcf-ui/.claude/skills/codegen/SKILL.md` around lines 36 - 39, Update the route-validation guidance in src/uis/nvcf-ui/.claude/skills/codegen/SKILL.md lines 36-39 and src/uis/nvcf-ui/.claude/skills/add-feature/SKILL.md lines 64-76 so it matches the configured Orval output: either enable and commit the Zod generator and document its generated APIs, or remove the nonexistent getWidgetsParams.zod example and use validation APIs actually produced by the current generator.Source: Coding guidelines
src/uis/nvcf-ui/backend/internal/middleware/middleware.go-49-57 (1)
49-57: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winApply
hlog.RequestIDHandlerto every request.
AddRequestIdbypasses the handler forGET, so downstream logs lackrequest_id. Remove the method branch and updateTestAddRequestIdto coverGETand assert the logger field.🤖 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/uis/nvcf-ui/backend/internal/middleware/middleware.go` around lines 49 - 57, Update AddRequestId to route every request through withRequestID, removing the GET-specific bypass so all downstream logs receive request_id. Extend TestAddRequestId to exercise a GET request and assert that the logger includes the request_id field.Sources: Coding guidelines, Path instructions
src/uis/nvcf-ui/THIRD-PARTY.txt-366-400 (1)
366-400: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftPin all four TanStack packages to exact versions.
Use
0.10.8,1.170.18,1.167.0, and0.8.1for@tanstack/react-devtools,@tanstack/react-router,@tanstack/react-router-devtools, and@tanstack/devtools-vite. Updatesrc/uis/nvcf-ui/ui/package.json, regeneratepnpm-lock.yaml, and record the same versions inTHIRD-PARTY.txt.🤖 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/uis/nvcf-ui/THIRD-PARTY.txt` around lines 366 - 400, Pin `@tanstack/react-devtools`, `@tanstack/react-router`, `@tanstack/react-router-devtools`, and `@tanstack/devtools-vite` to exact versions 0.10.8, 1.170.18, 1.167.0, and 0.8.1 respectively in the UI package configuration. Regenerate pnpm-lock.yaml and update the corresponding entries in THIRD-PARTY.txt to record those exact versions.src/uis/nvcf-ui/SECURITY.md-173-177 (1)
173-177: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftSecurity And Privacy (CWE-319): Cleartext Transmission of Sensitive Information
Exploitability: Difficult
Encrypt every BFF-to-upstream hop before sending
Authorization. The BFF injects bearer tokens into requests sent over hard-coded plaintexthttp. Use HTTPS or mTLS for each upstream, or enforce mesh encryption and peer authentication in the deployment.🤖 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/uis/nvcf-ui/SECURITY.md` around lines 173 - 177, Update the BFF upstream request configuration and deployment requirements to encrypt every BFF-to-upstream hop before transmitting Authorization tokens: replace plaintext HTTP with HTTPS or mTLS, or explicitly enforce service-mesh encryption and peer authentication for each upstream. Remove the assumption that cluster isolation alone provides transport confidentiality.src/uis/nvcf-ui/SECURITY.md-130-134 (1)
130-134: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftBroken Authentication (CWE-306): Missing Authentication for Critical Function
Exploitability: Trivial
Require authentication before enabling the
nvcfUiHTTPRoute.The BFF has no caller-authentication middleware and replaces incoming credentials with its own upstream bearer token. The chart creates an unauthenticated route when
nvcfUi.httpRoute.enabledis set totrue. Require an authenticating gateway or proxy before any external exposure.🤖 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/uis/nvcf-ui/SECURITY.md` around lines 130 - 134, Update the nvcfUi HTTPRoute enablement configuration so it cannot expose the BFF unless an authenticating gateway or proxy is explicitly configured. Ensure enabling nvcfUi.httpRoute.enabled requires that authentication prerequisite, and preserve the BFF’s existing service-token behavior.src/uis/nvcf-ui/helm/templates/_helpers.tpl-153-171 (1)
153-171: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
.Chart.Nameis used where a container name is required. The chart is namedhelm-nvcf-ui, and the Vault annotation helper passes.Chart.Nameas theagent-copy-volume-mountsvalue. That value must name a container in the pod, so the rendered value only works if a container is also namedhelm-nvcf-ui. If it does not match, the Vault Agent does not receive the projected OpenBao token mount and auto-auth fails.
src/uis/nvcf-ui/helm/templates/_helpers.tpl#L153-L171: replace{{ .Chart.Name }}on thevault.hashicorp.com/agent-copy-volume-mountsline with the literal app container name used indeployment.yaml.src/uis/nvcf-ui/helm/Chart.yaml#L16-L26: confirmname: helm-nvcf-uiis the intended chart name, since it also drivesnvcf-ui.nameandnvcf-ui.fullnameprefixes.🤖 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/uis/nvcf-ui/helm/templates/_helpers.tpl` around lines 153 - 171, The vaultAnnotations helper passes the chart name instead of the required pod container name to agent-copy-volume-mounts; replace .Chart.Name with the literal app container name used by deployment.yaml. In src/uis/nvcf-ui/helm/templates/_helpers.tpl lines 153-171, make that annotation target the app container so the projected token mount is copied correctly. In src/uis/nvcf-ui/helm/Chart.yaml lines 16-26, confirm that name: helm-nvcf-ui remains the intended chart name because it drives the nvcf-ui.name and nvcf-ui.fullname prefixes; no change is required there unless confirmation finds it incorrect.src/uis/nvcf-ui/helm/templates/_helpers.tpl-101-108 (1)
101-108: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd a distinct component label to the server selectors.
The server Deployment and Service select only shared labels, so they also match control-plane pods. Use
app.kubernetes.io/component: serverfor the server pod labels, Deployment selector, and Service selector.🤖 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/uis/nvcf-ui/helm/templates/_helpers.tpl` around lines 101 - 108, Update the server selector-label helper and all server pod, Deployment selector, and Service selector definitions to include app.kubernetes.io/component: server, while retaining the shared labels. Keep the existing nvcf-ui.controlPlane.selectorLabels component value unchanged so server and control-plane selectors remain distinct.src/uis/nvcf-ui/helm/templates/control-plane-rbac.yaml-26-30 (1)
26-30: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-732): Incorrect Permission Assignment for Critical Resource
Reachability: Internal
Scope ConfigMap access to
control-plane-health.The Role grants mutation access to every ConfigMap in the release namespace, including the mounted components and Vault Agent ConfigMaps. The production monitor uses only
Getand server-sideApplyfor ConfigMaps. Keepcreateunrestricted, restrictget,update, andpatchwithresourceNames: ["control-plane-health"], and removelistandwatch.🤖 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/uis/nvcf-ui/helm/templates/control-plane-rbac.yaml` around lines 26 - 30, Update the ConfigMap rule in the RBAC manifest to remove list and watch, retain unrestricted create, and split get, update, and patch into a rule scoped with resourceNames ["control-plane-health"].src/uis/nvcf-ui/helm/templates/rbac.yaml-26-29 (1)
26-29: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winExcessive Kubernetes RBAC Permissions (CWE-269): Improper Privilege Management
Reachability: Internal · Exploitability: Moderate
Restrict ConfigMap permissions to
control-plane-health.The server reads only the named
control-plane-healthConfigMap. This Role grantsget,list, andwatchon every ConfigMap in the release namespace. AddresourceNames: ["control-plane-health"]and removelistandwatch.🤖 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/uis/nvcf-ui/helm/templates/rbac.yaml` around lines 26 - 29, Update the ConfigMap rule in the RBAC template to target only the control-plane-health resource by adding resourceNames: ["control-plane-health"], and restrict verbs to get by removing list and watch.src/uis/nvcf-ui/backend/internal/control-plane/httphealth.go-41-47 (1)
41-47: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftInstrument outbound health checks.
m.httpClientis a plainhttp.Clientwith no OpenTelemetry wrapper. IncheckEndpoints, create a child span for each request, inject W3C trace context, and mark transport errors and non-2xx responses witherror=trueandotel.status_code=ERROR.🤖 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/uis/nvcf-ui/backend/internal/control-plane/httphealth.go` around lines 41 - 47, Update checkEndpoints to create and end a child OpenTelemetry span for each outbound health-check request, inject W3C trace context into the request headers, and preserve the span across the HTTP call. Mark transport errors and non-2xx responses with error=true and otel.status_code=ERROR before returning the existing wrapped errors.Source: Coding guidelines
src/uis/nvcf-ui/backend/cmd/server/main.go-121-138 (1)
121-138: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSet server timeouts and bound the shutdown.
The
http.Serverhas no read, write, or idle timeouts. A slow client can hold connections open and exhaust server resources.Shutdownalso usescontext.Background(), so a stuck in-flight proxy request blocks termination until the kubelet kills the pod.🛡️ Proposed fix
server := http.Server{ Addr: fmt.Sprintf("0.0.0.0:%d", port), Handler: handler, + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: 120 * time.Second, + IdleTimeout: 120 * time.Second, } @@ <-ctx.Done() defer tokenWatcher.Wait() - if err := server.Shutdown(context.Background()); err != nil { + shutdownCtx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + if err := server.Shutdown(shutdownCtx); err != nil { logger.Fatal().Err(err).Msg("Failed to shutdown server") }🤖 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/uis/nvcf-ui/backend/cmd/server/main.go` around lines 121 - 138, Update the http.Server initialization in the server startup flow to configure appropriate read, write, and idle timeouts, then replace the unbounded context.Background() passed to server.Shutdown with a bounded shutdown context and ensure it is released. Keep the existing shutdown trigger and error logging behavior intact.Source: Linters/SAST tools
src/uis/nvcf-ui/backend/internal/control-plane/k8shealth.go-47-97 (1)
47-97: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winZero the workload object before each Get; stale status can report a failed workload as healthy.
Each wrapper is constructed once in
Loadcomponentsand reused for the process lifetime.object()returns a pointer to the same embedded struct on every cycle, and the typed client decodes the API response into it without clearing it first. Status fields carryomitempty, so a field that becomes zero is absent from the response and keeps its previous value.Failure sequence for a Deployment:
- Cycle 1 returns
replicas: 2, readyReplicas: 2.replicasReady()passes.- Cycle 2 the workload has no ready pods, so the response omits
readyReplicas.- After decode
Status.ReadyReplicasis still 2,replicasReady()returns nil, and the component is reported healthy.Reset the embedded object in
object()so each cycle decodes into a clean struct.🐛 Proposed fix
-func (w *deploymentWorkload) object() client.Object { return &w.d } +func (w *deploymentWorkload) object() client.Object { + w.d = appsv1.Deployment{} + return &w.d +} @@ -func (w *statefulSetWorkload) object() client.Object { return &w.s } +func (w *statefulSetWorkload) object() client.Object { + w.s = appsv1.StatefulSet{} + return &w.s +} @@ -func (w *daemonSetWorkload) object() client.Object { return &w.d } +func (w *daemonSetWorkload) object() client.Object { + w.d = appsv1.DaemonSet{} + return &w.d +} @@ -func (w *replicaSetWorkload) object() client.Object { return &w.r } +func (w *replicaSetWorkload) object() client.Object { + w.r = appsv1.ReplicaSet{} + return &w.r +}🤖 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/uis/nvcf-ui/backend/internal/control-plane/k8shealth.go` around lines 47 - 97, Reset each embedded workload object before it is returned for decoding by updating the object methods on deploymentWorkload, statefulSetWorkload, daemonSetWorkload, and replicaSetWorkload. Ensure each method zeroes its stored Deployment, StatefulSet, DaemonSet, or ReplicaSet before returning its pointer, while preserving the existing client.Object return behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 394bbceb-88e2-4944-8a1d-05906a381caa
⛔ Files ignored due to path filters (70)
MODULE.bazel.lockis excluded by!**/*.lock,!**/MODULE.bazel.locksrc/uis/nvcf-ui/backend/go.sumis excluded by!**/*.sumsrc/uis/nvcf-ui/ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/uis/nvcf-ui/ui/public/favicon.icois excluded by!**/*.icosrc/uis/nvcf-ui/ui/src/generated/api/account/account.msw.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/account/account.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/clusters/clusters.msw.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/clusters/clusters.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/control-plane/control-plane.msw.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/control-plane/control-plane.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/function-deployment/function-deployment.msw.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/function-deployment/function-deployment.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/function-management/function-management.msw.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/api/function-management/function-management.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/accountDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/artifactDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/autoscalingConfigurationDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/containerEnvironmentEntryDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/controlPlaneComponentStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/controlPlaneComponentStatusStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/deploymentHealthDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/deploymentResponse.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionDeploymentDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionDeploymentDtoFunctionStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionDtoApiBodyFormat.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionDtoFunctionType.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionDtoStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionModelDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/functionResponse.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getAllFunctionsParams.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getAllFunctionsVisibilityItem.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponse.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponseCloudProvider.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponseClusterConfigurationFiles.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponseClusterConfigurations.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponseClusterUpgradeStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponseGpuUsage.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClusterResponseStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getClustersParams.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/getFunctionVersionParams.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/gpuCapacity.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/gpuResponseSchema.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/gpuSpecificationDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/gpuSpecificationDtoConfiguration.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/healthDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/healthDtoProtocol.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/helmValidationPolicyDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/helmValidationPolicyDtoName.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/imageConfig.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/imageCredentialHelper.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/instanceDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/instanceDtoInstanceStatus.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/instanceTypeResponseSchema.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/kubernetesType.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/listAccountResponse.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/listDeploymentsResponse.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/listFunctionsResponse.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/llmConfigDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/rateLimitDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/rateLimitDtoPerNcaIdRate.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/scalingDetails.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/sisConfig.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/stickinessWindow.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/telemetriesDto.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/src/generated/model/vaultConfig.tsis excluded by!**/generated/**src/uis/nvcf-ui/ui/vendor/kui-foundations/.versionis excluded by!**/vendor/**src/uis/nvcf-ui/ui/vendor/kui-foundations/base-external.cssis excluded by!**/vendor/**src/uis/nvcf-ui/ui/vendor/kui-foundations/components.cssis excluded by!**/vendor/**tools/collect-dependencies/go.sumis excluded by!**/*.sum
📒 Files selected for processing (198)
.bazelignore.github/CODEOWNERS.github/actions/bazel-java-build/action.yml.github/workflows/bazel.ymlMODULE.bazelNOTICEdependencies.mdgo.work.bazelrules/oci/defs.bzlrules/oci/private/go_web.bzlsrc/clis/nvcf-cli/internal/client/client_test.gosrc/clis/nvcf-cli/internal/state/state.gosrc/uis/nvcf-ui/.claude/skills/add-feature/SKILL.mdsrc/uis/nvcf-ui/.claude/skills/codegen/SKILL.mdsrc/uis/nvcf-ui/.claude/skills/run/SKILL.mdsrc/uis/nvcf-ui/.claude/skills/testing/SKILL.mdsrc/uis/nvcf-ui/.coderabbit.yamlsrc/uis/nvcf-ui/.gitignoresrc/uis/nvcf-ui/.security-triage.yamlsrc/uis/nvcf-ui/AGENTS.mdsrc/uis/nvcf-ui/BUILD.bazelsrc/uis/nvcf-ui/CLAUDE.mdsrc/uis/nvcf-ui/LICENSEsrc/uis/nvcf-ui/NOTICEsrc/uis/nvcf-ui/README.mdsrc/uis/nvcf-ui/SECURITY.mdsrc/uis/nvcf-ui/THIRD-PARTY.txtsrc/uis/nvcf-ui/Taskfile.ymlsrc/uis/nvcf-ui/backend/.gitignoresrc/uis/nvcf-ui/backend/.golangci.ymlsrc/uis/nvcf-ui/backend/BUILD.bazelsrc/uis/nvcf-ui/backend/Taskfile.ymlsrc/uis/nvcf-ui/backend/cmd/control-plane/BUILD.bazelsrc/uis/nvcf-ui/backend/cmd/control-plane/main.gosrc/uis/nvcf-ui/backend/cmd/server/BUILD.bazelsrc/uis/nvcf-ui/backend/cmd/server/main.gosrc/uis/nvcf-ui/backend/go.modsrc/uis/nvcf-ui/backend/internal/control-plane/BUILD.bazelsrc/uis/nvcf-ui/backend/internal/control-plane/handler.gosrc/uis/nvcf-ui/backend/internal/control-plane/handler_test.gosrc/uis/nvcf-ui/backend/internal/control-plane/health.gosrc/uis/nvcf-ui/backend/internal/control-plane/health_test.gosrc/uis/nvcf-ui/backend/internal/control-plane/heartbeat.gosrc/uis/nvcf-ui/backend/internal/control-plane/heartbeat_test.gosrc/uis/nvcf-ui/backend/internal/control-plane/httphealth.gosrc/uis/nvcf-ui/backend/internal/control-plane/k8shealth.gosrc/uis/nvcf-ui/backend/internal/control-plane/k8shealth_test.gosrc/uis/nvcf-ui/backend/internal/middleware/BUILD.bazelsrc/uis/nvcf-ui/backend/internal/middleware/middleware.gosrc/uis/nvcf-ui/backend/internal/middleware/middleware_test.gosrc/uis/nvcf-ui/backend/internal/token-watcher/BUILD.bazelsrc/uis/nvcf-ui/backend/internal/token-watcher/watcher.gosrc/uis/nvcf-ui/backend/internal/token-watcher/watcher_test.gosrc/uis/nvcf-ui/backend/internal/utils/BUILD.bazelsrc/uis/nvcf-ui/backend/internal/utils/utils.gosrc/uis/nvcf-ui/backend/internal/utils/utils_test.gosrc/uis/nvcf-ui/biome.jsonsrc/uis/nvcf-ui/helm/.helmignoresrc/uis/nvcf-ui/helm/Chart.yamlsrc/uis/nvcf-ui/helm/templates/NOTES.txtsrc/uis/nvcf-ui/helm/templates/_helpers.tplsrc/uis/nvcf-ui/helm/templates/control-plane-components-configmap.yamlsrc/uis/nvcf-ui/helm/templates/control-plane-deployment.yamlsrc/uis/nvcf-ui/helm/templates/control-plane-rbac.yamlsrc/uis/nvcf-ui/helm/templates/control-plane-serviceaccount.yamlsrc/uis/nvcf-ui/helm/templates/deployment.yamlsrc/uis/nvcf-ui/helm/templates/hook-openbao-migrations.yamlsrc/uis/nvcf-ui/helm/templates/hpa.yamlsrc/uis/nvcf-ui/helm/templates/httproute.yamlsrc/uis/nvcf-ui/helm/templates/rbac.yamlsrc/uis/nvcf-ui/helm/templates/referencegrant.yamlsrc/uis/nvcf-ui/helm/templates/service.yamlsrc/uis/nvcf-ui/helm/templates/serviceaccount.yamlsrc/uis/nvcf-ui/helm/templates/vault-agent-configmap.yamlsrc/uis/nvcf-ui/helm/values.yamlsrc/uis/nvcf-ui/license-header.txtsrc/uis/nvcf-ui/spec/control-plane-openapi.yamlsrc/uis/nvcf-ui/spec/nvcf-openapi.yamlsrc/uis/nvcf-ui/spec/sis-openapi.yamlsrc/uis/nvcf-ui/tools/workspace_status.shsrc/uis/nvcf-ui/ui/.gitignoresrc/uis/nvcf-ui/ui/BUILD.bazelsrc/uis/nvcf-ui/ui/Taskfile.ymlsrc/uis/nvcf-ui/ui/biome.jsonsrc/uis/nvcf-ui/ui/index.htmlsrc/uis/nvcf-ui/ui/orval.config.tssrc/uis/nvcf-ui/ui/package.jsonsrc/uis/nvcf-ui/ui/pnpm-workspace.yamlsrc/uis/nvcf-ui/ui/public/mockServiceWorker.jssrc/uis/nvcf-ui/ui/scripts/vendor-kui-css.jssrc/uis/nvcf-ui/ui/src/components/AsyncBoundary.tsxsrc/uis/nvcf-ui/ui/src/components/CodeSnippet.tsxsrc/uis/nvcf-ui/ui/src/components/CopyButton.tsxsrc/uis/nvcf-ui/ui/src/components/CopyText.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/ActiveFilters.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/ColumnVisibility.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Content.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/DataTable.test.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/DataTable.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/DataTableContext.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Filters.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/ItemCount.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Pagination.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Search.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Skeleton.test.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Skeleton.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Sort.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/Toolbar.tsxsrc/uis/nvcf-ui/ui/src/components/DataTable/index.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/columns.test.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/columns.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/filter-fns.test.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/filter-fns.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/filter-items.test.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/filter-items.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/index.tssrc/uis/nvcf-ui/ui/src/components/DataTable/utils/tableSearchSchema.tssrc/uis/nvcf-ui/ui/src/components/DetailList.test.tsxsrc/uis/nvcf-ui/ui/src/components/DetailList.tsxsrc/uis/nvcf-ui/ui/src/components/NotFound.tsxsrc/uis/nvcf-ui/ui/src/components/OverflowGroup.test.tsxsrc/uis/nvcf-ui/ui/src/components/OverflowGroup.tsxsrc/uis/nvcf-ui/ui/src/components/RouteErrorFallback.tsxsrc/uis/nvcf-ui/ui/src/components/RouteSpinner.tsxsrc/uis/nvcf-ui/ui/src/config/.gitkeepsrc/uis/nvcf-ui/ui/src/features/accounts/components/AccountSwitcher.test.tsxsrc/uis/nvcf-ui/ui/src/features/accounts/components/AccountSwitcher.tsxsrc/uis/nvcf-ui/ui/src/features/accounts/components/AccountSwitcherSkeleton.tsxsrc/uis/nvcf-ui/ui/src/features/accounts/getActiveNcaId.tssrc/uis/nvcf-ui/ui/src/features/accounts/hooks/useActiveAccount.tssrc/uis/nvcf-ui/ui/src/features/clusters/ClusterDetails.test.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/ClusterDetails.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/ClustersList.test.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/ClustersList.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/ClustersListPending.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/components/ClustersPageHeading.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/components/GpuUtilization.test.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/components/GpuUtilization.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/components/StatusBadge.test.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/components/StatusBadge.tsxsrc/uis/nvcf-ui/ui/src/features/clusters/constants.tssrc/uis/nvcf-ui/ui/src/features/clusters/routes.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/Dashboard.test.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/Dashboard.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/ClusterStatsPanel.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/ControlPlaneStatsPanel.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/FunctionStatsPanel.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/PageHeading.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/ReadyClustersPanel.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/RecentFunctionsList.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/StatPanelBoundary.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/components/StatPanelSkeleton.tsxsrc/uis/nvcf-ui/ui/src/features/dashboard/routes.tsxsrc/uis/nvcf-ui/ui/src/features/functions/FunctionDetail.test.tsxsrc/uis/nvcf-ui/ui/src/features/functions/FunctionDetail.tsxsrc/uis/nvcf-ui/ui/src/features/functions/FunctionsList.test.tsxsrc/uis/nvcf-ui/ui/src/features/functions/FunctionsList.tsxsrc/uis/nvcf-ui/ui/src/features/functions/FunctionsListPending.tsxsrc/uis/nvcf-ui/ui/src/features/functions/components/FunctionCard.test.tsxsrc/uis/nvcf-ui/ui/src/features/functions/components/FunctionCard.tsxsrc/uis/nvcf-ui/ui/src/features/functions/components/FunctionsPageHeading.tsxsrc/uis/nvcf-ui/ui/src/features/functions/components/StatusBadge.tsxsrc/uis/nvcf-ui/ui/src/features/functions/constants.tssrc/uis/nvcf-ui/ui/src/features/functions/hooks/useFunctionsWithDeployments.tssrc/uis/nvcf-ui/ui/src/features/functions/routes.tsxsrc/uis/nvcf-ui/ui/src/features/functions/types/index.tssrc/uis/nvcf-ui/ui/src/hooks/useLocalStorage.test.tssrc/uis/nvcf-ui/ui/src/hooks/useLocalStorage.tssrc/uis/nvcf-ui/ui/src/hooks/useTableUrlSync.test.tssrc/uis/nvcf-ui/ui/src/hooks/useTableUrlSync.tssrc/uis/nvcf-ui/ui/src/hooks/useThemePreference.test.tsxsrc/uis/nvcf-ui/ui/src/hooks/useThemePreference.tssrc/uis/nvcf-ui/ui/src/lib/fetch.test.tssrc/uis/nvcf-ui/ui/src/lib/fetch.tssrc/uis/nvcf-ui/ui/src/lib/queryClient.tssrc/uis/nvcf-ui/ui/src/main.tsxsrc/uis/nvcf-ui/ui/src/mocks/browser.tssrc/uis/nvcf-ui/ui/src/mocks/handlers.tssrc/uis/nvcf-ui/ui/src/mocks/scenarios/functions/all-statuses.tssrc/uis/nvcf-ui/ui/src/mocks/scenarios/functions/empty-list.tssrc/uis/nvcf-ui/ui/src/mocks/scenarios/functions/no-deployment.tssrc/uis/nvcf-ui/ui/src/mocks/server.tssrc/uis/nvcf-ui/ui/src/mocks/store/handlers.tssrc/uis/nvcf-ui/ui/src/mocks/store/index.tssrc/uis/nvcf-ui/ui/src/mocks/store/store.tssrc/uis/nvcf-ui/ui/src/rootRoute.tsxsrc/uis/nvcf-ui/ui/src/router.tsxsrc/uis/nvcf-ui/ui/src/styles.csssrc/uis/nvcf-ui/ui/src/testing/render.tsxsrc/uis/nvcf-ui/ui/src/types/table.d.tssrc/uis/nvcf-ui/ui/src/utils/clipboard.test.tssrc/uis/nvcf-ui/ui/src/utils/clipboard.tssrc/uis/nvcf-ui/ui/src/utils/formatters.test.tssrc/uis/nvcf-ui/ui/src/utils/formatters.tssrc/uis/nvcf-ui/ui/src/utils/resize-observer.tssrc/uis/nvcf-ui/ui/tsconfig.jsonsrc/uis/nvcf-ui/ui/vite.config.tssrc/uis/nvcf-ui/ui/vitest.setup.ts
| ], | ||
| entrypoint = ["/app/server"], | ||
| registry = "nvcr.io/0651155215864979/ncp-dev/nvcf-ui", | ||
| source_layer = "//:source_layer", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for a root-package source_layer target.
set -uo pipefail
rg -n 'name = "source_layer"' --glob 'BUILD.bazel' --glob '*.bzl' .
sed -n '1,80p' BUILD.bazel 2>/dev/null | rg -n 'source_layer' || echo "root BUILD.bazel has no source_layer"Repository: NVIDIA/nvcf
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- guidance files ---'
find .. -name AGENTS.md -print
printf '%s\n' '--- BUILD.bazel context ---'
cat -n src/uis/nvcf-ui/BUILD.bazel | sed -n '55,100p'
printf '%s\n' '--- root BUILD.bazel ---'
if [ -f BUILD.bazel ]; then
cat -n BUILD.bazel | sed -n '1,120p'
else
echo 'No root BUILD.bazel'
fi
printf '%s\n' '--- source_layer references ---'
rg -n --glob 'BUILD.bazel' --glob '*.bzl' 'source_layer|nvcf-ui' .Repository: NVIDIA/nvcf
Length of output: 12670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository guidance ---'
cat AGENTS.md
printf '%s\n' '--- nvcf-ui guidance ---'
cat src/uis/nvcf-ui/AGENTS.md
printf '%s\n' '--- rule definition and implementation ---'
cat -n rules/oci/private/go_web.bzl | sed -n '25,145p'Repository: NVIDIA/nvcf
Length of output: 32260
Reference the local source_layer target.
//:source_layer resolves to the root package, which has no such target. Use :source_layer to reference //src/uis/nvcf-ui:source_layer; otherwise Bazel analysis fails with no such target.
🤖 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/uis/nvcf-ui/BUILD.bazel` at line 86, Update the source_layer attribute in
the nvcf-ui BUILD target to use the local :source_layer label instead of
//:source_layer, ensuring it resolves to the package’s source_layer target.
Why
Closes #694.
Sanjay observed nv-boot-parent building alongside the services that depend on
it. Measuring three runs shows it is worse than concurrent: it reliably
finishes last.
Two things combine. The matrix is built from
find src -name bazel-java-ci.json | LC_ALL=C sort, which orderssrc/libraries/java/nv-boot-parentaftersrc/control-plane-services/*, so theframework is the last of five rows. The docker-host lane caps at
max-parallel: 4. The framework therefore cannot start until a service frees aslot, and the services compile against whatever the cache already held rather
than the library they were meant to.
What changed
The framework row runs as its own job; the service rows depend on it.
The gate is conditional. The framework job only exists when change detection
selected it, which is exactly when nv-boot-parent changed. When it did not, the
job is skipped and the services start immediately, so ordinary runs pay nothing.
To avoid duplicating a 153-line job, its steps move to a composite action at
.github/actions/bazel-java-build. Both jobs are thin callers:The steps were extracted mechanically rather than retyped:
matrix.subtree.*became
inputs.*, and everyrun:step gained the explicitshell:thatcomposite actions require.
Customer Release Notes
Not customer visible.
Plan Summary
Not applicable.
Usage
Not applicable.
Testing
actionlintclean on the workflow. The composite action parses and every one ofits five
run:steps was verified to carry an explicitshell:; a missing onefails only at runtime.
Not yet exercised on a branch that actually changes nv-boot-parent, which is
the run that proves the gate fires. That is the next step and I am not claiming
it here.
Notes
always()on the service job is required rather than stylistic. A skippeddependency would otherwise skip every service, and skipped is the common case.
The required-checks aggregator now evaluates the framework result. Without it a
framework failure would skip the service rows and the aggregate would pass on
their skipped state.
On the DoD: this is not the unconditional prebuild the issue proposed. The
measurement showed the problem is ordering, not a missing warm-up stage, so the
cost is paid only on nv-boot-parent changes rather than every run. The duplicate
NV Boot compilation figure is still unmeasured; if it turns out large, a warm
cache from this gate should show up as shorter service rows on framework
changes.
References
Closes #694
Related Merge Requests/Pull Requests
None
Dependencies
None
Github commit:
ci(bazel): gate Java services on the framework row when nv-boot-parent changes
Closes #694
Co-authored-by: Balaji Ganesan bganesan@nvidia.com
Summary by CodeRabbit
New Features
Documentation