feat(onboarding): Docker label discovery during init - #6158
Open
ajnart wants to merge 5 commits into
Open
Conversation
Port label-based container discovery from config-portability and wire it into the onboarding integrations step. Labeled containers take priority over image matching, sync into category sections, and preserve board layout when placing integration widgets.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Docker label discovery and sync for onboarding, backed by a new ChangesDocker label discovery, sync, and onboarding
Sequence Diagram(s)sequenceDiagram
participant OnboardRouter
participant SyncService
participant Database
OnboardRouter->>SyncService: discovered label apps
SyncService->>Database: load boards and existing sources
SyncService->>Database: insert or update apps, items, layouts
SyncService->>Database: upsert docker_app_source rows
SyncService-->>OnboardRouter: created / updated / skipped counts
sequenceDiagram
participant InitIntegrations
participant SyncLabelApps
participant CreateApps
InitIntegrations->>InitIntegrations: split selected apps by source
InitIntegrations->>SyncLabelApps: label-sourced apps
InitIntegrations->>CreateApps: image-sourced apps
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@apps/docs/docs/integrations/docker-labels/index.mdx`:
- Around line 32-35: The Homepage fallback documentation is incomplete because
`homepage.icon` and `homepage.description` are missing from the fallback payload
description. Update the `Homepage fallback` section in the Docker labels docs to
list all fields handled by the fallback behavior, using the existing
`homepage.*` symbols alongside `homarr.name` so the reference fully matches the
implementation.
In `@apps/docs/docs/integrations/docker/index.mdx`:
- Around line 96-100: The Docker labels documentation link in the onboarding
section points to the wrong relative location and will resolve to a 404 from
this page. Update the link in the Docker integration docs to point to the
sibling docker-labels page using the correct relative path, and verify the
markdown link in the Homarr onboarding copy still references the intended Docker
labels reference.
In `@apps/nextjs/src/app/`[locale]/init/_steps/integrations/init-integrations.tsx:
- Around line 161-170: The label-app sync flow in init-integrations currently
filters out missing hosts and still continues into setup, which can hide a sync
result of notFound. Update the logic around syncLabelApps and the
labelAppsWithHost handling so missing label apps are explicitly detected and
surfaced before calling setupIntegrations(), and stop completion or prompt the
user when selected apps no longer exist. Use the
syncDockerLabelApps/syncLabelApps path and the labelAppsWithHost block to locate
the change.
- Around line 386-388: The continue flow in
handleContinueToConfig/init-integrations should treat app-only selections as a
valid path to completion instead of always entering ConfigurePhase. Update the
transition logic so that when selectedKinds is empty but selectedAppCount is
greater than zero, it skips the configure step and goes straight to the finish
path, avoiding any access to selectedKinds[currentIndex] in ConfigurePhase. Keep
the existing behavior for kind-based selections, but guard the configure-phase
branch against empty selectedKinds.
In `@packages/api/src/router/onboard/onboard-router.ts`:
- Around line 190-191: `onboard-router` is mutating `emptyResult` by aliasing
its `integrations` and `apps` arrays into `discoveredIntegrations` and
`discoveredApps`, so later failures can return populated data with `status:
"unavailable"`. In the onboarding flow, make `discoveredIntegrations` and
`discoveredApps` fresh local arrays instead of references to `emptyResult`, and
only merge them into the success response path; keep `emptyResult` unchanged so
the `catch` path in `onboard-router` always spreads truly empty fallback data.
In `@packages/db/schema/sqlite.ts`:
- Around line 559-563: The unique identity in this table is currently based on
compoundKey using host and containerId, but syncDiscoveredServicesAsync and
lookups rely on host plus externalId. Update the primary uniqueness in the table
definition from containerId to externalId so existing mappings are reused when
containers are recreated, and keep containerId as non-unique mutable metadata;
also verify the externalIdIdx and any related schema references in sqlite.ts
still align with the host/externalId identity.
In `@packages/docker/src/discovery/parse-container-labels.ts`:
- Around line 37-45: Normalize blank values to undefined in readLabel so
whitespace-only labels do not count as present before fallback logic runs.
Update readLabel and the resolveLabelValue path in parse-container-labels.ts so
homarr.* keys that trim to an empty string are treated as missing, allowing
homepage.* fallback and the container.Id default to work correctly for fields
like name and externalId.
In `@packages/docker/src/discovery/sync-discovered-services.ts`:
- Around line 233-236: The update paths in sync-discovered-services do not
refresh updatedAt when modifying an existing source row, so the timestamp stays
stale even though the row is active. Update the relevant update statements in
the discovery sync flow (including the blocks around the source row updates) to
explicitly set updatedAt to the current time alongside the existing
appId/boardId/itemId/externalId fields, using the same database update logic
already used in this module.
- Around line 136-153: The new item’s vertical placement is being computed only
once via getNextItemYOffset using firstLayout.id and then reused across all
layouts, which can cause overlaps when breakpoints differ. Update
sync-discovered-services.ts in the item layout creation flow so each entry in
board.layouts.map computes its own yOffset using that layout’s id before
building the item layout object. Keep the fix localized around
getNextItemYOffset, firstLayout, and the newItemLayouts mapping.
- Around line 341-343: The ping status toggle logic in sync-discovered-services
is letting service.pingUrl override options.forceDisableStatus, so
forceDisableStatus is ignored for services with a ping URL. Update the
pingEnabled assignment in the service sync flow to check forceDisableStatus
first and only enable pinging when it is not disabled, then apply the existing
enableStatusByDefault/pingUrl behavior afterward. Use the pingEnabled logic near
the service discovery mapping in sync-discovered-services to make the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c93dfc58-3cbd-4c0a-a2b3-edd4b91ea369
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/docs/docs/integrations/docker-labels/index.mdxapps/docs/docs/integrations/docker-labels/index.tsapps/docs/docs/integrations/docker/index.mdxapps/nextjs/src/app/[locale]/init/_steps/integrations/init-integrations.tsxpackages/api/src/router/onboard/onboard-router.tspackages/db/migrations/mysql/0042_add_docker_app_source.sqlpackages/db/migrations/mysql/meta/_journal.jsonpackages/db/migrations/postgresql/0008_add_docker_app_source.sqlpackages/db/migrations/postgresql/meta/_journal.jsonpackages/db/migrations/sqlite/0040_add_docker_app_source.sqlpackages/db/migrations/sqlite/meta/_journal.jsonpackages/db/schema/index.tspackages/db/schema/mysql.tspackages/db/schema/postgresql.tspackages/db/schema/sqlite.tspackages/docker/package.jsonpackages/docker/src/discovery/__tests__/parse-container-labels.spec.tspackages/docker/src/discovery/index.tspackages/docker/src/discovery/list-discovered-containers.tspackages/docker/src/discovery/parse-container-labels.tspackages/docker/src/discovery/sync-discovered-services.tspackages/docker/src/discovery/types.tspackages/docker/src/index.tspackages/docker/src/labels.tspackages/server-settings/src/index.ts
Fix app-only continue flow, handle stale label sync, per-layout yOffset, blank label normalization, updatedAt on source upsert, docs link, and forceDisableStatus for ping-enabled apps.
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
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
homarr.*/homepage.*) from the config-portability work into onboardingdocker_app_sourcetrackingTest plan
parse-container-labels.spec.ts(28 tests)@homarr/docker,@homarr/apihomarr.name/group/hrefappear in onboarding with correct metadataSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests