Conversation
This release branch snapshots the latest dev work onto current main so the update center can surface recent non-stable Docker tags without operators looking them up manually. The branch keeps stable Docker discovery as the primary path while carrying the new recent-tag auto-discovery, UI affordances, and regression coverage through the repo's release flow. Constraint: Publication must start from latest origin/main and merge origin/dev on a clean release branch Constraint: Stable Docker Hub discovery semantics must remain intact while adding recent non-stable discovery Rejected: Re-open or reuse the already-merged PR #494 branch | it has already been absorbed by main and deleted remotely Rejected: Publish directly from dev | repo workflow requires a clean release branch to main Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep future dev-tag publication work on fresh release branches cut from origin/main; do not revive merged release branches Tested: Focused update-center vitest suite, npm run typecheck, npm run repo:drift-check, npm run build:web, npm run docs:build, git diff --check (release branch) Not-tested: Full npm test remains red because tmp/sub2api frontend suites are still broken in this repo snapshot
github-actions
Bot
added
area: server
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdateCenterStatus is extended with optional ChangesDocker Hub Recent Tags Normalization & Filtering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/server/routes/api/updateCenter.test.ts (1)
265-275: Consider an explicit empty-recentNonStableassertion.This scenario returns
recentNonStable: [], but the responsetoMatchObjecton Line 294-303 doesn't assert the absence/emptiness ofdockerHubRecentTagsin the API response. AddingdockerHubRecentTags: []to the expectation would lock in that an empty list is faithfully returned (vs. omitted or replaced withnull), which matters for the UI that iterates this field unconditionally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/routes/api/updateCenter.test.ts` around lines 265 - 275, In the "returns partial status when a single version source lookup fails" test, update the toMatchObject assertion (the response JSON asserted after calling the route) to explicitly include dockerHubRecentTags: [] so the test verifies an empty array is returned; locate the test by its description and the mocks (fetchLatestStableGitHubReleaseMock, fetchDockerHubTagCandidatesMock) and modify the expectation that currently uses toMatchObject to add the dockerHubRecentTags: [] key/value pair.src/web/pages/settings/UpdateCenterSection.test.tsx (1)
459-501: New deploy-from-recent-tag test is meaningful but the card locator is fragile.The assertion flow (locate card → click auto-deploy button → verify
deployUpdateCenterpayload → verify log stream) is good coverage for the new one-click path. However, the card locator on Lines 473-476 picks the first element with anystyleobject whose rendered text contains the target string — that can match an ancestor (e.g., the whole section wrapper) rather than the intended tag card, silently masking rendering regressions. Prefer a more specific predicate (e.g., adata-testid, a class substring likedocker-hub-recent-tag-card, or matching the immediate deploy-button container).♻️ Suggested tightening
- const recentTagCard = root.root.find((node) => ( - typeof node.props?.style === 'object' - && collectText(node).includes('dev-20260417-f67ade2 @ sha256:bbbbbbbbbbbb') - )); + const recentTagCard = root.root.find((node) => ( + typeof node.props?.className === 'string' + && node.props.className.includes('docker-hub-recent-tag') // or a data-testid + && collectText(node).includes('dev-20260417-f67ade2 @ sha256:bbbbbbbbbbbb') + ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/web/pages/settings/UpdateCenterSection.test.tsx` around lines 459 - 501, The test's card locator is too broad and may match an ancestor; tighten the predicate in UpdateCenterSection.test.tsx (the "deploys auto-discovered recent non-stable Docker Hub tags without manual input" test) so it targets the specific tag card element instead of any node with a style object — for example, look for a node with a dedicated identifier like data-testid or a class substring (e.g., 'docker-hub-recent-tag-card'), or require the node to be a container element that also contains the deploy button child (the same element that yields deployRecentButton) before asserting and clicking; update the find used to set recentTagCard to this stricter predicate so the subsequent deployRecentButton click and apiMock assertions remain valid.src/server/services/updateCenterVersionService.ts (1)
142-155: Alias detection is case-sensitive while priority is case-insensitive — minor inconsistency.
isPreferredDockerHubAliascompares against['latest','main']without lowercasing, butgetRecentNonStableDockerHubPrioritydoes lowercase the tag. In the unlikely case of a tag pushed asLatest/Main, it would slip pastisStableDockerHubTagand end up inrecentNonStablewith priority 3. Low impact in practice (Docker Hub tags are conventionally lowercase), but trivial to align for consistency.♻️ Suggested alignment
function isPreferredDockerHubAlias(input: string | null | undefined): boolean { - const tag = normalizeDockerHubTagName(input); + const tag = normalizeDockerHubTagName(input).toLowerCase(); return PREFERRED_DOCKER_HUB_TAG_ALIASES.includes(tag as typeof PREFERRED_DOCKER_HUB_TAG_ALIASES[number]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/services/updateCenterVersionService.ts` around lines 142 - 155, The alias check is currently case-sensitive (isPreferredDockerHubAlias uses normalizeDockerHubTagName but compares against PREFERRED_DOCKER_HUB_TAG_ALIASES as-is) while other logic lowercases tags (getRecentNonStableDockerHubPriority), so make alias detection case-insensitive by normalizing case: update normalizeDockerHubTagName to return the trimmed lowercase string (or alternatively lowerCase the tag inside isPreferredDockerHubAlias before comparing), and ensure isPreferredDockerHubAlias compares the lowercased tag against the lowercased alias list (or against the existing constants if normalize now lowercases) so 'Latest'/'Main' are treated the same as 'latest'/'main'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/web/pages/settings/UpdateCenterSection.tsx`:
- Around line 224-227: The filter in normalizeRecentDockerCandidates currently
allows entries missing a real Docker tag by accepting entries with only
normalizedVersion; update the predicate so it only returns entries that have a
non-empty raw Docker tag (check entry?.tagName is a non-blank string) to ensure
the UI can only enable one-click deploys when a real tag is present; modify the
function normalizeRecentDockerCandidates (operating on
UpdateCenterStatus['dockerHubRecentTags']) to require and trim entry.tagName
rather than allowing normalizedVersion.
- Around line 817-823: The UI currently shows candidateLabel and publishedAt but
not the candidateDigest that runDeploy will use; update the JSX around the
candidate display (the divs rendering candidateLabel and the publishedAt hint in
UpdateCenterSection.tsx) to render the candidateDigest (e.g., a short/truncated
digest string or a copyable full digest) before the deploy controls so operators
can see which digest will be deployed; ensure the digest value referenced is the
same prop/variable named candidateDigest and leave the existing runDeploy call
unchanged.
---
Nitpick comments:
In `@src/server/routes/api/updateCenter.test.ts`:
- Around line 265-275: In the "returns partial status when a single version
source lookup fails" test, update the toMatchObject assertion (the response JSON
asserted after calling the route) to explicitly include dockerHubRecentTags: []
so the test verifies an empty array is returned; locate the test by its
description and the mocks (fetchLatestStableGitHubReleaseMock,
fetchDockerHubTagCandidatesMock) and modify the expectation that currently uses
toMatchObject to add the dockerHubRecentTags: [] key/value pair.
In `@src/server/services/updateCenterVersionService.ts`:
- Around line 142-155: The alias check is currently case-sensitive
(isPreferredDockerHubAlias uses normalizeDockerHubTagName but compares against
PREFERRED_DOCKER_HUB_TAG_ALIASES as-is) while other logic lowercases tags
(getRecentNonStableDockerHubPriority), so make alias detection case-insensitive
by normalizing case: update normalizeDockerHubTagName to return the trimmed
lowercase string (or alternatively lowerCase the tag inside
isPreferredDockerHubAlias before comparing), and ensure
isPreferredDockerHubAlias compares the lowercased tag against the lowercased
alias list (or against the existing constants if normalize now lowercases) so
'Latest'/'Main' are treated the same as 'latest'/'main'.
In `@src/web/pages/settings/UpdateCenterSection.test.tsx`:
- Around line 459-501: The test's card locator is too broad and may match an
ancestor; tighten the predicate in UpdateCenterSection.test.tsx (the "deploys
auto-discovered recent non-stable Docker Hub tags without manual input" test) so
it targets the specific tag card element instead of any node with a style object
— for example, look for a node with a dedicated identifier like data-testid or a
class substring (e.g., 'docker-hub-recent-tag-card'), or require the node to be
a container element that also contains the deploy button child (the same element
that yields deployRecentButton) before asserting and clicking; update the find
used to set recentTagCard to this stricter predicate so the subsequent
deployRecentButton click and apiMock assertions remain valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85d1da34-6fd9-42ee-9959-d4eb921e8f33
📒 Files selected for processing (10)
docs/k3s-update-center.mdsrc/server/routes/api/updateCenter.test.tssrc/server/services/updateCenterPollingService.test.tssrc/server/services/updateCenterRuntimeStateService.test.tssrc/server/services/updateCenterRuntimeStateService.tssrc/server/services/updateCenterStatusService.tssrc/server/services/updateCenterVersionService.test.tssrc/server/services/updateCenterVersionService.tssrc/web/pages/settings/UpdateCenterSection.test.tsxsrc/web/pages/settings/UpdateCenterSection.tsx
Review feedback pointed out two small but important operator-facing gaps in the recent-tag card: one-click deploy should only be available when a real raw Docker tag exists, and the digest that will be deployed should be visible on the card instead of remaining implicit. This patch tightens the candidate filter to require tagName and renders the digest metadata before the deploy actions, with regression coverage for both behaviors. Constraint: The deploy API still requires a raw Docker tag, not a normalized display value Constraint: The recent-tag card should surface the exact digest operators are about to deploy Rejected: Keep accepting normalizedVersion-only candidates | could enable one-click deploy with a non-tag display value Rejected: Rely on displayVersion alone for digest visibility | digest presence should stay explicit even if display text changes later Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future recent-tag UI should treat tagName as the deployable identity and digest as separately visible metadata Tested: src/web/pages/settings/UpdateCenterSection.test.tsx, npm run typecheck, git diff --check Not-tested: Full PR checks rerun pending after push
The release branch had fallen behind current main and only conflicted in the Update Center recent-tag test. Keep the latest main-side deploy-state coverage while preserving the PR's raw-tag filtering and digest visibility assertions. Constraint: PR #495 must merge into the current main without widening the PR diff beyond the update-center candidate behavior. Rejected: Drop the PR-specific digest/raw-tag assertions | would regress the review fixes that prompted the branch update. Confidence: high Scope-risk: narrow Directive: Keep recent Docker tag cards deployable only from real tagName values and keep digest metadata visible before deployment. Tested: npx vitest run --root . src/web/pages/settings/UpdateCenterSection.test.tsx src/server/services/updateCenterVersionService.test.ts src/server/routes/api/updateCenter.test.ts src/server/services/updateCenterStatusService.test.ts; npm run typecheck; npm run repo:drift-check; git diff --cached --check origin/main Not-tested: Full npm test.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
github-actions
Bot
added
size: XS
cita-777
deleted the
release/feat-update-center-recent-docker-tags-20260418
branch
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
1 participant
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
This PR upgrades the update center's Docker Hub discovery path so operators do not need to manually look up recent dev / branch / sha tags before deploying.
What changed
latest/main/ stable SemVer) as the primary candidateVerification
Passed on
devand again on the clean release branch:src/server/services/updateCenterVersionService.test.tssrc/server/services/updateCenterRuntimeStateService.test.tssrc/server/services/updateCenterPollingService.test.tssrc/server/routes/api/updateCenter.test.tssrc/web/pages/settings/UpdateCenterSection.test.tsxnpm run typechecknpm run repo:drift-checknpm run build:webnpm run docs:buildgit diff --checkNotes
npm testis still red in this repo snapshot because unrelatedtmp/sub2api/frontend/**suites are broken/missing dependencies; this PR does not change that baseline.Summary by CodeRabbit
Bug Fixes
New Features