feat: show disconnected email inboxes in the UI - #397
Closed
Melvin0070 wants to merge 3 commits into
Closed
Conversation
OAuth and IMAP inboxes could silently stop receiving mail when a token was revoked or credentials were rejected, with a log line as the only signal. Persist a per-inbox connection status and surface it so admins can reconnect. - Add nullable inboxes.disconnected_at column (migration v2.5.0). - Flag the inbox on IMAP auth / OAuth-refresh failure and clear it on the next successful auth, via a ConnectionStatusCallback mirroring the existing token refresh callback. Writes only on a transition, seeded from the DB on load. - Show a warning badge in the inbox table and a reconnect banner in the inbox edit form when disconnected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a ChangesInbox Disconnection Detection
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant IMAP as imap.go
participant Email as Email.setConnectionStatus
participant Manager as inbox.Manager
participant DB as Database
participant UI as InboxList / EmailInboxForm
IMAP->>Email: authentication succeeds or fails
Email->>Email: check if status transitioned
alt error occurred
Email->>Manager: ConnectionStatusCallback(inboxID, err)
Manager->>DB: MarkDisconnected -> set-inbox-disconnected
else authentication succeeds
Email->>Manager: ConnectionStatusCallback(inboxID, nil)
Manager->>DB: MarkConnected -> set-inbox-connected
end
UI->>DB: fetch inbox (disconnected_at)
DB-->>UI: return disconnected_at value
UI->>UI: render InboxConnectionStatus / OAuth alert state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for surfacing “disconnected” email inboxes (IMAP + OAuth) so admins can see and act on inbox auth/connection loss without relying on logs.
Changes:
- Adds
inboxes.disconnected_at(nullable) and DB helpers to set/clear it. - Tracks/persists email inbox connection health from the email channel (IMAP auth / OAuth refresh paths) into the inbox manager.
- Updates the admin UI to display a disconnected badge in the inbox list and show a reconnect-style banner in the email inbox form, with new i18n strings.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| schema.sql | Adds disconnected_at column to inboxes. |
| internal/migrations/v2.5.0.go | Migration to add disconnected_at to existing DBs. |
| internal/inbox/queries.sql | Extends inbox selects + adds set/clear disconnected queries. |
| internal/inbox/models/models.go | Exposes disconnected_at on the inbox model (DB/JSON). |
| internal/inbox/inbox.go | Adds manager methods + prepared statements for setting/clearing disconnected state. |
| internal/inbox/channel/email/imap.go | Hooks connection-status updates into IMAP auth/refresh flow. |
| internal/inbox/channel/email/email.go | Adds connection-status callback plumbing + transition logic. |
| cmd/init.go | Wires email inbox initialization to seed and persist disconnected state. |
| cmd/upgrade.go | Registers the new v2.5.0 migration. |
| i18n/en-US.json | Adds strings for disconnected status/banners. |
| frontend/apps/main/src/views/admin/inbox/InboxList.vue | Adds a “Status” column using the new status component. |
| frontend/apps/main/src/features/admin/inbox/InboxConnectionStatus.vue | New component to display disconnected badge + tooltip. |
| frontend/apps/main/src/features/admin/inbox/EmailInboxForm.vue | Shows OAuth reconnect banner styling/messages when disconnected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+113
to
+130
| e.connStatusMu.Lock() | ||
| disconnected := connErr != nil | ||
| if disconnected == e.disconnected { | ||
| e.connStatusMu.Unlock() | ||
| return | ||
| } | ||
| e.disconnected = disconnected | ||
| e.connStatusMu.Unlock() | ||
|
|
||
| if disconnected { | ||
| e.lo.Warn("inbox disconnected from mail server", "inbox_id", e.id, "error", connErr) | ||
| } else { | ||
| e.lo.Info("inbox reconnected to mail server", "inbox_id", e.id) | ||
| } | ||
|
|
||
| if err := e.connStatusCallback(e.id, connErr); err != nil { | ||
| e.lo.Error("failed to persist inbox connection status", "inbox_id", e.id, "error", err) | ||
| } |
Comment on lines
93
to
99
| if e.authType == imodels.AuthTypeOAuth2 && e.oauth != nil { | ||
| // Refresh OAuth token if needed | ||
| oauthConfig, _, err := e.refreshOAuthIfNeeded() | ||
| if err != nil { | ||
| e.setConnectionStatus(err) | ||
| return err | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/inbox/channel/email/email.go`:
- Around line 106-131: The cached connection state in Email.setConnectionStatus
is updated before connStatusCallback succeeds, so a failed persistence leaves
e.disconnected out of sync and blocks future retries. Move the state commit in
setConnectionStatus so e.disconnected is only updated after connStatusCallback
returns nil, while keeping the transition check and logging around
Email.setConnectionStatus, e.connStatusMu, and e.connStatusCallback intact.
🪄 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 Plus
Run ID: 793dac33-d308-4034-a607-ea8f576b564b
📒 Files selected for processing (13)
cmd/init.gocmd/upgrade.gofrontend/apps/main/src/features/admin/inbox/EmailInboxForm.vuefrontend/apps/main/src/features/admin/inbox/InboxConnectionStatus.vuefrontend/apps/main/src/views/admin/inbox/InboxList.vuei18n/en-US.jsoninternal/inbox/channel/email/email.gointernal/inbox/channel/email/imap.gointernal/inbox/inbox.gointernal/inbox/models/models.gointernal/inbox/queries.sqlinternal/migrations/v2.5.0.goschema.sql
Address review feedback on abhinavxd#397: - Cache the disconnected flag only after the DB write succeeds, so a failed persist is retried on the next poll instead of being swallowed by the transition check. - Don't flag a disconnect on transient transport errors while refreshing the OAuth token; only credential rejections from the provider count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
|
Thanks for the reviews — both findings addressed in 70fe307:
|
Upstream claimed the v2.5.0 migration slot (per-type drafts), so the disconnected_at migration moves to v2.6.0. No other conflicts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Hey, thanks for the PR. I am working whatsapp channel side by side, I ended up fixing this issue there. |
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.
Closes #388.
OAuth (Google/Microsoft) and IMAP inboxes can silently stop receiving mail when a token is revoked or credentials are rejected — today the only signal is a log line. This surfaces that state so admins know to reconnect.
Approach
TokenRefreshCallbackpattern (email channel → inbox manager). Only credential failures flip it; transient dial/network errors are left alone so the indicator doesn't flap. Status is tracked in memory and seeded from the DB on load, so it writes only on a real transition, not on every poll.inboxes.disconnected_atcolumn (migrationv2.5.0). I kept this out of theconfigJSONB since that's encrypted and rewritten on every update, and a column is cleaner to surface/query. Happy to move it into config JSON instead if you'd prefer no schema change.Testing
go build ./...,go vet, the email package tests,eslint, andpnpm build:mainall pass. I wasn't able to exercise a real token revocation end-to-end; glad to walk through any manual verification you'd like.Notes
en-US.jsonis touched (source locale); other languages come through Crowdin.imap.go,models.go,EmailInboxForm.vue), so whichever lands second will need a rebase.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes