feat: improve Dozzle Cloud discoverability

[amir20]

Summary

• Add cloud.yaml as first-class config, separate from notification dispatchers — with automatic migration from existing notifications.yml
• Add cloud icon to header (between bell and settings) with 3-state popover: unlinked (value prop + link CTA), connected (plan/usage status), error (re-link CTA)
• Add Dozzle Cloud card to settings page showing connection status or linking CTA
• Add side-by-side Cloud/Webhook cards on notifications page empty state for better destination discovery
• Support from query param in OAuth callback to route users back to where they started (cloud popup vs notifications page)

Test plan

• Start Dozzle with existing cloud dispatcher in notifications.yml — verify migration creates cloud.yml and strips inline API key
• Start Dozzle without cloud.yml — verify grey cloud icon appears, popover shows value prop
• Link instance from cloud icon — verify redirect with from=cloud, return to /#cloudLinked
• Link instance from notifications page — verify redirect with from=notifications, return to notifications
• Verify settings page shows cloud card with plan/usage when linked
• Verify unlink from settings card deletes cloud.yml and cloud dispatchers
• Verify notifications empty state shows Cloud + Webhook cards

🤖 Generated with Claude Code

[claude]

Critical:

• Data race (multi_host_service.go): cloudConfig *notification.CloudConfig is read/written from concurrent HTTP handlers with no mutex. CloudConfig(), SetCloudConfig(), RemoveCloudConfig() need synchronization.

• Bug (CloudSettingsCard.vue:371-382): doUnlink() clears UI state regardless of fetch response status. If the DELETE fails, the user sees an unlinked UI but the server config is unchanged.

  await fetch(withBase("/api/cloud/config"), { method: "DELETE" });
  cloudConfig.value = null; // clears even on network error

Minor:

• onClosed() in CloudPopover.vue:188-190 is an empty no-op — safe to remove.
• fetchCloudConfig logic is duplicated across CloudPopover.vue, CloudSettingsCard.vue, and notifications.vue. A shared composable would eliminate the 3x duplication.

Looks good:

• Migration from notifications.yml is clean; RemoveDispatcher already calls saveNotificationConfig() so persistence is fine.
• from param handling in cloudCallback is safe (falls through to default).
• Linked: true hard-coded in cloudConfigResponse is correct since the handler only runs when cc != nil.

[amir20]

Where do the notifications get updated to cloud? Is that already done?

[amir20]

Doesn't this need to broadcast to all agents?

[amir20]

If this is migrated then why is it needed?

[claude]

Critical:

• Migration from notifications.yml is missing. PR description says "automatic migration from existing notifications.yml" but no migration code exists. Existing users with a cloud dispatcher in notifications.yml will silently lose it on upgrade — createDispatcher now skips "cloud" type with a warning, and LoadConfig ignores apiKey/prefix/expiresAt fields. cloud.yml will never be created for them.

Bugs:

• CloudSettingsCard.vue:doUnlink(): on DELETE failure, cloudStatusError.value = true is set but the modal stays open with no visible error — the error alert is rendered in the parent template, not inside the <dialog>.
• CloudSettingsCard.vue:cloudLinkUrl hardcodes from=cloud; linking from the settings page will redirect to /#cloudLinked (home) instead of back to settings. Should be from=settings.

Performance:

• useCloudConfig() is not a shared composable — CloudPopover.vue, CloudSettingsCard.vue, DestinationForm.vue, and alertForm.ts each get independent state and make separate /api/cloud/config fetches on mount. Consider createSharedComposable(useCloudConfig) from VueUse.

Minor:

• usagePercent computed can return Infinity if events_limit === 0.
• setTimeout in CloudPopover.vue:onMounted isn't cleared on unmount (onUnmounted(() => clearTimeout(...))).

[claude]

Review: feat: improve Dozzle Cloud discoverability

Bugs

• Migration non-atomicity (migration/cloud_config.go): If writing cloud.yml succeeds but rewriting notifications.yml fails, cloud.yml now exists (skipping future runs) while notifications.yml still holds the cloud APIKey + remapped subscription IDs — unrecoverable inconsistency. Consider writing to a temp file + rename, or at minimum bail out and delete cloud.yml if the second write fails.

• Shared error state bleed (CloudSettingsCard.vue doUnlink): On DELETE failure, sets shared cloudStatusError=true from useCloudConfig(), causing CloudPopover and CloudDestinationForm to show an error state they didn't trigger.

Issues

• Duplicate fetch (cloudConfig.ts fetchCloudConfig): No in-flight guard. CloudPopover and CloudSettingsCard both call fetchCloudConfig() in onMounted — opening settings fires two concurrent fetches. fetchCloudStatus has the isLoadingCloudStatus guard; add the same for fetchCloudConfig.

• Unused i18n keys (locales/en.yml): empty-state.cloud-subtitle and empty-state.webhook-subtitle are defined but not referenced in notifications.vue. Either wire them up or remove.

Minor

• cloudLinkUrl and usagePercent are duplicated identically in CloudPopover and CloudSettingsCard — expose from useCloudConfig().
• DispatcherConfig.Prefix has yaml:"-" which is correct, but a short comment explaining why (cloud-only, not persisted here) would help.

[amir20]

Hmm it feels weird that this part of notification to update cloud? Should cloud be it's own standalone method?

[amir20]

Yea, similar question I wonder if this should outside of notification manager?

[amir20]

Does it ever get broadcasted?

[amir20]

Oh good, it doest have it's own class. So above there is just some mix with notification?

[claude]

Review

Migration partial-write risk (internal/migration/cloud_config.go:86-102)
If writeYAML(cloudPath, ...) creates the file but encoding fails midway, cloud.yml exists (empty/corrupt) and fileExists(cloudPath) prevents re-migration on restart. Subscriptions were remapped and the cloud dispatcher removed from the in-memory config before the failed write — inconsistent state with no recovery path. Consider writing to a temp file and atomically renaming.

Dead prop path (assets/components/Notification/DestinationCard.vue:44-58)
DestinationCard still defines and forwards existingDispatchers to DestinationForm, but DestinationForm dropped that prop. notifications.vue still passes :existing-dispatchers too. No functional bug (Vue silently ignores it via attrs), but it is dead code.

Double fetchCloudConfig on settings page (assets/composable/cloudConfig.ts)
Both CloudPopover and CloudSettingsCard call fetchCloudConfig() on mount. On the settings page both mount simultaneously, firing two identical GET /api/cloud/config requests. A guard like 'if (cloudConfig.value) return' at the top of fetchCloudConfig would fix it.

Unguarded write in StartNotificationManager (internal/support/docker/multi_host_service.go:225)
m.cloudConfig = &cc is written without holding cloudMu. Safe at startup since the service is not shared yet, but inconsistent with every other write site.

Minor: cloudConfig handler returns linked: true whenever the config exists, even if APIKey is empty (e.g. corrupt YAML decoded without error). Consider gating on cc.APIKey != empty string.

[claude]

Review

Migration atomicity (internal/migration/cloud_config.go): If the first writeYAML (cloud.yml) succeeds but the second (notifications.yml) fails, both files exist with the cloud dispatcher in notifications.yml. On next startup migration is skipped (cloud.yml exists), leaving a stale cloud dispatcher entry until the next WriteConfig call. Low probability but a comment or TODO would help.

Broadcast scope on new host (multi_host_service.go): When a new agent connects, broadcastCloudConfig() and broadcastNotificationConfig() send to all agents, not just the new one. For deployments with many agents this causes N-1 redundant RPCs per connection event.

Host channel buffer (multi_host_service.go): hostCh has buffer 1. Two agents coming online near-simultaneously could drop one event, leaving that agent without its initial config broadcast.

No migration tests: MigrateCloudConfig has no unit tests for the subscription remap logic or partial-failure paths. Given it mutates config files at startup, basic table tests would be worth adding.

linked field always true (internal/web/cloud.go cloudConfigResponse): The Linked field is hardcoded to true whenever a config exists — the frontend checks cloudConfig?.linked but this can never be false. Either remove the field or document the invariant.

Minor: double blank line after ClearCloudDispatcher in manager.go.

[claude]

Review: feat: improve Dozzle Cloud discoverability

Bugs / Issues

• cloudConfig.ts:589 — No in-flight dedup in fetchCloudConfig. CloudPopover and CloudSettingsCard both call it on onMounted, so mounting both concurrently fires two parallel fetches. Simple fix: add an isLoading flag or a pending-promise ref.

• CloudSettingsCard.vue:337 — On unlink failure, cloudStatusError.value = true but the modal stays open with no visible error message. User gets no feedback that unlink failed.

• cloud.go:2374 — Linked: true is hardcoded in cloudConfigResponse. The field is always true when the 200 is returned (nil config returns 404). Either drop the field or derive it from a real state check.

• CloudSettingsCard.vue:309 — cloudLinkUrl uses from=cloud, so re-linking from the settings page redirects to /#cloudLinked (root) instead of staying on settings. Test plan covers the popover flow but not the settings card re-link path.

Non-critical

• migration/cloud_config.go:1636 — Migration is non-atomic: if the process crashes after writing cloud.yml but before rewriting notifications.yml, the old cloud dispatcher remains. The fileExists guard prevents re-running but leaves a stale entry.

• manager.go:1844 — Extra blank line.

• notifications.vue — DestinationCard still receives :existing-dispatchers prop in the non-empty branch. Confirm DestinationCard still accepts it or remove the binding.

[claude]

Issues:

• cloudConfig.ts:25 — fetchCloudStatus has no guard against concurrent calls (if (isLoadingCloudStatus.value) return). Both CloudPopover and CloudSettingsCard call it on mount, so the settings page fires two parallel requests.

• CloudSettingsCard.vue:14 — cloudLinkUrl hardcodes &from=cloud, so OAuth callback redirects to /#cloudLinked even when triggered from the settings page. Should use from=settings to return the user to /settings#cloudLinked.

• CloudSettingsCard.vue:118-125 — doUnlink sets cloudStatusError.value = true on failure with no user-facing feedback (no toast). Leaves the UI in a confusing state.

• internal/migration/cloud_config.go — no tests for the subscription ID remapping (lines 87-91). Silent migration failure here would break existing cloud subscriptions without error surface. Worth a unit test.

• internal/notification/manager.go:323 — double blank line before getDispatcher.

[claude]

Review: feat/cloud-discoverability

Architecture is clean — separating cloud config from notifications.yml is the right call.

Issues:

• agent_command.go:64 — SetCloudDispatcher type-asserts to *dispatcher.CloudDispatcher to read APIKey/Prefix/ExpiresAt. If the assertion fails (wrong impl passed), persistence silently drops with no log. The UpdateCloudConfig gRPC already carries the full config struct — better to pass notification.CloudConfig directly to SetCloudDispatcher instead of re-extracting it from the dispatcher.

• migration/cloud_config.go:102 and multi_host_service.go:saveCloudConfig — both write directly to the target file via os.Create. A crash or write error mid-stream leaves the file empty/truncated. Use write-to-temp + os.Rename for atomic replacement (especially critical for notifications.yml rewrite during migration).

• web/cloud.go:177 — json.NewEncoder(w).Encode(resp) error unchecked in cloudConfig handler.

• Last commit (68d2cbac) is titled "debug: add broadcast cloud config logging" — should either be squashed into the feature commit or the log level dropped before merge.

Minor nits:

• Double blank line in manager.go after ClearCloudDispatcher (cosmetic).
• CloudSettingsCard.vue:doUnlink sets cloudStatusError.value = true on DELETE failure but the modal stays open, leaving the error state visible with no feedback to the user — worth adding a toast or inline error.

[claude]

Security — API key file permissions

• os.Create(cloudConfigPath) and os.Create("./data/cloud.yml") use default mode 0666 (0644 after typical umask) — readable by group/world. The cloud API key should be 0600.
• multi_host_service.go (saveCloudConfig), agent_command.go (SetCloudDispatcher): replace os.Create with os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)

Migration atomicity (internal/migration/cloud_config.go:100-113)

• If writeYAML(notificationsPath, ...) fails after cloud.yml is already written, next startup skips migration (cloud.yml exists) but old notifications.yml still has the cloud dispatcher. LoadConfig would load it again → duplicate cloud dispatcher. Consider writing to a temp file and renaming, or at minimum logging a warning if rewrite fails.

Pointer-to-interface (internal/notification/manager.go:24)

• atomic.Pointer[dispatcher.Dispatcher] stores a pointer to an interface, which is unusual. Both Store(&d) and (*p) dereferences work correctly but obscure intent. sync.Value or atomic.Pointer[CloudDispatcher] would be clearer.

Minor

• usagePercent in both CloudPopover.vue and CloudSettingsCard.vue: division by events_limit could be Infinity if limit is 0 (edge case on free tier with no quota).
• Double blank line in manager.go after ClearCloudDispatcher.
• cloudConfig.ts: concurrent fetchCloudConfig() calls from multiple components aren't deduplicated; a simple in-flight flag would prevent redundant requests on mount.