feat: add notification inbox triage#224
Conversation
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
Capture product decisions for an Octobox-style Inbox backed by GitHub notifications, including local done state, closed-item completion, background GitHub read propagation, and separate notification sync cadence.
Incorporate roborev design review feedback by tightening Inbox state semantics, bootstrap sync behavior, propagation retry contracts, monitored-repo visibility, API generation dependencies, and frontend ownership boundaries.
Add an Octobox-style Inbox backed by GitHub notifications, including SQLite storage, Huma APIs, generated clients, GitHub notification ingestion, queued read propagation, and closed linked-item completion. Wire the Inbox into the Svelte UI with filters, selection-based bulk actions, route navigation, design-system examples, and store/API coverage. Notification sync now runs on separate configurable intervals while manual sync also triggers notification ingestion.
Address PR review feedback by reporting missing notification IDs in bulk responses, continuing notification processing across host failures, and refreshing the inbox after sync triggers. Adds backend, store, and Playwright coverage for the reviewed notification flows.
Restyle the notification inbox as a compact full-height panel instead of a card stack so it matches the activity view feedback. Add e2e coverage that keeps notification rows in an internal scroll region.
Tighten the inbox layout regression so the fixture actually overflows the notification list and verifies the app shell remains non-scrolling.
Address roborev feedback by preserving queued local read state during GitHub refreshes, adding per-host notification sync watermarks with periodic full reconciliation, scoping API visibility to active tracked repos, and marking notifications done when linked PRs or issues close. Adds DB/server/sync coverage for the notification state transitions.
Track background notification sync state so failed refreshes are visible through the inbox API and UI polling instead of returning a silent 202. Route GitHub Enterprise PR notifications to their web URL until hosted PR detail routing exists. The base-path server test helper now shuts down its server before temp DB cleanup so sync requests launched during CSRF tests cannot leave background work racing directory removal. Verified with:\n- go test ./... -short -shuffle=on\n- bun test packages/ui/src/stores/notifications.test.ts packages/ui/src/utils/notificationRoutes.test.ts\n- cd packages/ui && bun run typecheck\n- git diff --check
Gate both the explicit notification sync endpoint and the global sync follow-up on the notifications.enabled config flag, so disabled installations do not call the GitHub notifications API manually. Verified with:\n- go test ./internal/server -run 'TestNotificationsSyncEndpointRejectsWhenDisabled|TestGlobalSyncSkipsNotificationsWhenDisabled|TestNotificationsAPIExposesBackgroundSyncStatus' -shuffle=on\n- go test ./... -short -shuffle=on\n- git diff --check
Move notification sync status onto the syncer so endpoint, global, and periodic notification refreshes share the same running/error state. Make notification summary counts ignore the active list state while preserving the other filters so inbox counters reflect unread, active, and done totals consistently. Verified with:\n- go test ./internal/db -run '^TestNotificationSummaryIgnoresListState$' -shuffle=on\n- go test ./internal/server -run 'TestGlobalSyncExposesNotificationSyncFailure|TestNotificationsAPIExposesBackgroundSyncStatus|TestNotificationsSyncEndpointRejectsWhenDisabled|TestGlobalSyncSkipsNotificationsWhenDisabled' -shuffle=on\n- go test ./internal/github -run 'Notification|SyncNotifications' -shuffle=on\n- go test ./... -short -shuffle=on\n- git diff --check
Include platform_host in notification repo summary facets so same owner/name repos on different hosts do not collapse together. Fetch participating notification IDs through GitHub's participating filter and merge that metadata into synced notification rows. Verified with:\n- go test ./internal/db -run 'TestNotificationSummaryRepoFacetsIncludePlatformHost|TestNotificationsHideUnmonitoredRepos|TestNotificationSummaryIgnoresListState' -shuffle=on\n- go test ./internal/github -run 'TestSyncNotificationsMarksParticipatingThreads|TestSyncNotificationsUsesPersistedSinceWatermark|TestSyncNotificationsDoesPeriodicFullSyncForReadState|TestSyncNotificationsClearsSinceWhenTrackedReposChange|TestNotificationItem' -shuffle=on\n- go test ./internal/server -run 'TestNotificationsAPIUsesActiveTrackedRepos|TestGlobalSyncExposesNotificationSyncFailure' -shuffle=on\n- go test ./... -short -shuffle=on\n- git diff --check
Notification repo facets now include platform_host, so the API accepts those same host-qualified facet keys when filtering the inbox. Verified with:\n- go test ./internal/server -run 'TestNotificationsAPIAcceptsHostQualifiedRepoFilter|TestNotificationsAPIUsesActiveTrackedRepos' -shuffle=on\n- go test ./internal/db -run 'TestNotificationSummaryRepoFacetsIncludePlatformHost|TestNotificationsHideUnmonitoredRepos' -shuffle=on\n- go test ./... -short -shuffle=on\n- git diff --check
Reject non-positive notification ticker intervals during config validation so the notification loops cannot panic. Send priority sort from the inbox store so mentions and review requests are ordered ahead of lower-signal notifications. Verified with:\n- go test ./internal/config -run '^TestLoadRejectsNonPositiveNotificationIntervals$' -shuffle=on\n- bun test packages/ui/src/stores/notifications.test.ts\n- cd packages/ui && bun run typecheck\n- go test ./... -short -shuffle=on\n- bun test packages/ui/src/stores/notifications.test.ts packages/ui/src/utils/notificationRoutes.test.ts\n- git diff --check
Scope notification read reconciliation by platform host so GitHub and GitHub Enterprise threads with matching IDs do not affect each other. Parse and build inbox filter query params, pass them into the inbox store, and write state/search/sort changes back to the route so triage views survive reloads and sharing. Verified with:\n- go test ./internal/db -run '^TestMarkNotificationsReadScopesThreadIDsToPlatformHost$' -shuffle=on\n- cd frontend && bun run test -- src/lib/stores/router.test.ts\n- cd frontend && bun run typecheck\n- cd packages/ui && bun test src/stores/notifications.test.ts src/utils/notificationRoutes.test.ts\n- cd packages/ui && bun run typecheck\n- go test ./... -short -shuffle=on\n- git diff --check
Avoid stale read propagation clearing newer unread notification activity by matching the queued attempt before recording success. Add visible reason, type, and repository inbox filters and cover them in the full-stack inbox e2e flow. Verified with:\n- go test ./internal/db -run 'Test(ReadPropagationSuccessDoesNotClearNewerUnreadActivity|NotificationsQueueReadPropagation)$' -shuffle=on\n- go test ./internal/github ./internal/server -run 'Notification' -shuffle=on\n- cd frontend && bun run test:e2e -- tests/e2e-full/inbox.spec.ts\n- go test ./... -short -shuffle=on\n- cd frontend && bun run test -- src/lib/stores/router.test.ts\n- cd frontend && bun run typecheck\n- cd packages/ui && bun test src/stores/notifications.test.ts src/utils/notificationRoutes.test.ts\n- cd packages/ui && bun run typecheck\n- git diff --check
Guard read propagation failures with the queued attempt identity so stale workers cannot attach retry metadata to newer unread activity. Return a notification loop stop handle and wait for in-flight sync or propagation work during shutdown before the syncer is torn down. Verified with:\n- go test ./cmd/middleman -run '^TestNotificationLoopStopWaitsForInFlightRun$' -shuffle=on\n- go test ./internal/db -run 'TestReadPropagation(FailureDoesNotMarkNewerUnreadActivityForRetry|SuccessDoesNotClearNewerUnreadActivity)$' -shuffle=on\n- go test ./internal/github ./internal/server -run 'Notification' -shuffle=on\n- go test ./... -short -shuffle=on (failed once: TestAPITriggerSyncIgnoresRequestCancellation TempDir cleanup; go test ./internal/server -short -shuffle=on passed on rerun)\n- git diff --check
When notification read propagation reaches the retry cap, persist max_attempts_exceeded without a next retry timestamp. Update inbox copy so terminal failures are not shown as automatically retrying. Verified with:\n- go test ./internal/github -run '^TestProcessQueuedNotificationReadsStopsRetryMetadataAtMaxAttempts$' -shuffle=on\n- go test ./... -short -shuffle=on\n- cd packages/ui && bun run typecheck\n- cd frontend && bun run typecheck && bun run lint\n- git diff --check
Poll notification sync until the completion timestamp advances or the retry budget expires, instead of stopping when the first reload still reports running=false. Disable rows without a notification destination and show link-unavailable copy. Verified with:\n- cd packages/ui && bun test src/stores/notifications.test.ts\n- cd packages/ui && bun run typecheck\n- git diff --check
Flush additional promise turns in the frontend Vitest path so the final scheduled notification poll resolves before call-count assertions. Verified with:\n- cd frontend && bun run test -- ../packages/ui/src/stores/notifications.test.ts\n- cd packages/ui && bun test src/stores/notifications.test.ts
Let notification background work run on the parent cancellation context instead of canceling at the ticker interval. Normalize PullRequest notification subjects that use issue-style API URLs so they still get PR numbers and internal links. Verified with:\n- go test ./cmd/middleman -run '^TestNotificationLoopStopWaitsForInFlightRun$' -shuffle=on\n- go test ./internal/github -run 'TestNotificationItem' -shuffle=on\n- go test ./... -short -shuffle=on\n- git diff --check
Add a script and make targets that clone the configured SQLite database into the worktree, rewrite a clone-local config, and run backend or frontend dev servers against that cloned data directory instead of the user's main database. Verified with:\n- bun test scripts/dev-clone-db.test.mjs\n- make script-tests\n- git diff --check
Make frontend-dev-clone-db reuse an existing cloned config when present so starting Vite does not replace the cloned SQLite file under a running clone backend. Verified with:\n- make script-tests\n- git diff --check
Run the cloned backend with buildvcs disabled so air can build from this worktree, and default the cloned frontend target to port 5175 to avoid common Vite port conflicts. Verified with:\n- make script-tests\n- git diff --check
The CSRF guard requires application/json on mutation requests, including zero-body endpoints. The inbox sync store now sends that header so the sync button triggers the background notification refresh instead of receiving 415. Verified with:\n- cd packages/ui && bun test src/stores/notifications.test.ts\n- cd packages/ui && bun run typecheck\n- git diff --check
roborev: Combined Review (
|
Strengthen PR tests so notification filters, sync refresh, CSRF headers, cloned config rewriting, and bulk API mutations assert visible outcomes instead of broad pass-through behavior. Add an e2e-only notification fixture hook so the inbox sync test can prove newly synced data appears after refresh.
roborev: Combined Review (
|
CI runs package tests through the frontend Vitest environment, where Request auto-adds text/plain for string bodies. Detect explicit caller headers before constructing the Request so implicit text/plain does not block the JSON content type required by mutation endpoints.
roborev: Combined Review (
|
Notification sync now fills missing authors from the locally synced PR or issue row so inbox metadata and author search work with real GitHub notification payloads. Individual PR/issue sync and direct close/merge handlers now mark linked notifications done as soon as closed state is persisted, instead of waiting for a later full repo or notification sync.
Keep notification host processing deterministic without map lookup nil ambiguity so the push-time nilaway hook accepts the sync path.
roborev: Combined Review (
|
The cloned database target copies private SQLite data into the worktree. Create the clone directory owner-only and chmod the cloned database/config files to 0600 so default umask settings cannot expose synced repository or notification data to other local users.
roborev: Combined Review (
|
Queued local reads no longer claim a GitHub last-read timestamp before propagation succeeds. The inbox type filter now exposes release, commit, and other notification subjects so non-PR/issue rows visible under All types can be isolated from the UI.
roborev: Combined Review (
|
1 participant
Summary