Conversation
|
@github-copilot review |
Follow-up audit completeAudited live head Feedback classification
Thermo-nuclear finding and RCAThe evaluator producer and delivery consumer independently formatted the queued Commit Product/UI dogfoodDogfooded the exact PR runtime on desktop (1280x720) and mobile (390x844): empty state, create, edit threshold, snooze, resume, disable, and delete all succeeded. The UI states the cost-saving purpose, provides a useful 10 errors / 5 minutes / 30-minute cooldown starting point, and keeps actions usable at both viewports. The committed Playwright scenario independently passed. Verification
No external blocker remains. |
|
Final follow-up on head
Final state: open, current with |
| } while (await results.NextPageAsync()); | ||
|
|
||
| var plan = RateNotificationCounterPlan.Compile(projectId, rules); | ||
| await _cache.SetAsync(cacheKey, plan, CacheTtl); |
There was a problem hiding this comment.
[P2] Prevent invalidation from losing to an in-flight cache fill
The striped gates only coordinate this process. In a multi-replica deployment, worker A can load the pre-mutation rules, API B can save and invalidate the shared key, and then A can publish its stale plan after that removal. The old plan remains usable for the five-minute TTL, so new rules can miss countering and disabled/deleted rules can remain active. Please coordinate load/publish and invalidation with a distributed per-project generation/CAS or lock so an invalidation cannot be overwritten by an older fill; add a regression that pauses the fill across a mutation.
| </div> | ||
|
|
||
| <RateNotificationRuleList | ||
| hasPremiumFeatures={selectedProject.has_premium_features} |
There was a problem hiding this comment.
[P2] Remove the unreachable premium branch tree
has_rate_notifications is already the canonical premium-plus-rollout capability, and both rate-rule components are mounted only inside that guard. Therefore hasPremiumFeatures is always true here: the upgrade callbacks, alerts, disabled branches, and the enabled argument used to build requests can never execute in production. Please remove that redundant plumbing from the page, list, form, schema helper, and its dead test so the combined capability has one owner and the two entitlement models cannot drift.
|
Thermo-nuclear follow-up on head 002497e: Commit 002497e applies the surgical review fixes: removes the resurrected migration cron resume, aligns the canonical email source with singular/plural rendering, reuses the E2E polling helper, restores PUT stack-scope validation parity, centralizes supported-window policy across API/runtime, makes active-key discovery precede increments, memoizes duplicate bucket reads, and aligns generated nullable-enum schemas with the API contract. Static evidence: git diff --check passed; OpenAPI and endpoint-manifest JSON parse; source/compiled template placeholders and generated TypeScript/Zod nullability agree. The contract generator could not run because this isolated worktree has no installed swagger-typescript-api. Per the explicit review constraint, no local tests, builds, Aspire, E2E, browser, runtime services, or dogfood were run, so these fixes are not locally test-verified. Two deeper non-surgical findings remain as inline review threads. |
58e3c30 to
ae336a0
Compare
What
Add personal email alerts when a project or stack reaches a configured event count within a time window. Users can create, edit, disable, snooze, and delete rules from account notification settings.
The ingestion pipeline shares minute counters across matching rules. Scheduled evaluation applies thresholds and cooldowns; delivery rechecks the rule, membership, email preferences, and project notification budget. Rule changes invalidate counter plans across replicas. Evaluation allows one minute for in-flight writes to settle, adding one minute of alert latency. Emails use the current typed Razor renderer.
Why
Occurrence notifications can miss sustained bursts. Rate rules let users choose when a burst warrants an email and control repeat alerts.
APIs and behavior
/api/v2/users/{userId}/projects/{projectId}/rate-notifications.rate-notificationsorganization feature. Rules remain stored after a downgrade or feature removal.has_rate_notificationsto project responses.Verification
mainatbb0315556.ae336a09d: 3,053 backend tests passed (three intentional skips), all 64 browser E2E tests passed including rule management, frontend checks passed, and Docker builds passed. CI run.Breaking changes
None. The feature is additive and gated.