fix: code review follow-ups - flush() race + extension/dashboard hardening#24
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
What
Addresses code review findings across the extension and dashboard: one real concurrency bug (duplicate events on sync) plus a set of small robustness/UX fixes. Also ships two UX additions: a clickable popup title and a dashboard footer linking to GitHub releases.
Changes
Code
extension/src/background/sync.ts— re-entry guard inflush()(the alarm and the inlineSESSION_STOPflush can no longer run concurrently) + re-read of the queue before slicing so events appended by content scripts during anawaitaren't dropped. Eliminates duplicate events in the DB.extension/src/background/index.ts— singledecodeJwtPayload()(oneatobparse instead of two);ensureDbSession()now logs failures instead of a silentvoid; extractedisTrackBlocked()(removed duplicated blocklist check in two places); exhaustiveness guard (never) on the message-type dispatcher.extension/src/popup/index.html+popup.css— theWORKTRACEtitle is now a link toworktrace-ecru.vercel.app.dashboard/app/dashboard/reports/report-form.tsx— localStorageQuotaExceededErroris no longer swallowed: it surfaces as a toast.dashboard/app/dashboard/reports/api-key-modal.tsx— validate the Groq key by itsgsk_prefix + inline hint, instead of a length-only check.dashboard/app/dashboard/layout.tsx— footer with a link to GitHub releases (styled like the header:bg-bg/95 backdrop-blur).Tooling / config
Docs / plan
Design decisions
flushingboolean. The service worker is single-threaded, so a synchronous check before the firstawaitis enough to serialise — no need for a persistent lock in storage.pendingEventsbeforeslice. Sent events are always at the head of the queue; new ones are appended to the tail by content scripts writing directly to storage, so we only dropsentCountfrom the fresh state.never) on message types — a new message type now triggers a compile error instead of being silently ignored.gsk_validation — reject an invalid key early, before encryption/storage; the backend remains the source of truth (Groq auth error).Verification
npm run typecheck(extension) — clean (npx tsc --noEmit)npm run lint(dashboard) — clean (lint-staged on commit)Out of scope (future PRs)
.max()limits on eventcontent/tags+ a cap on the number of events in report-context.withCors.Closes