feat(notifications): referral invite nudge banner at SessionStart#243
Conversation
A one-time SessionStart banner telling signed-in users they earn $20 of org credit for inviting teammates (pairs with the deeplake-api referral program). - new rules/referral-invite.ts: fires once, from the 3rd session on (skips the first two), only when signed in; stable dedupKey shows it a single time. Kill switch (REFERRAL_NUDGE_ENABLED) for dark-shipping — on, since referral is live. - session counter in notifications-state.json: bumpSessionCount advances once per session, deduped by session_id (the two parallel SessionStart fires don't double-count); state read/markShown now preserve the counter fields. - hook entry registers the rule, bumps the counter, passes sessionCount into the rule context (rules stay IO-free). Tests: rule cadence/sign-in gating + bumpSessionCount (increment, dedup, missing-id). Pre-existing "built artifact" welcome bundle tests are unrelated (fail on clean main).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds persistent session counting to notifications, increments the count once per distinct ChangesNotification Session Cadence Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 5 files changed
Generated for commit 609ab70. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/claude-code/notifications-referral-invite.test.ts (1)
40-41: ⚡ Quick winStrengthen the “past 3rd session” assertion with payload identity.
At Line 41,
not.toBeNull()validates firing but not that the correct notification is returned. AtoMatchObject({ id: "referral-invite", dedupKey: { v: 1 } })check would better lock contract behavior.🤖 Prompt for 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. In `@tests/claude-code/notifications-referral-invite.test.ts` around lines 40 - 41, Replace the loose null-check with a strict payload identity assertion: call referralInviteRule.evaluate(ctx({ creds: signedIn, sessionCount: 9 })) and assert the returned object matches the expected notification shape (e.g., toMatchObject({ id: "referral-invite", dedupKey: { v: 1 } })) so the test verifies both firing and the exact notification payload rather than only non-null.
🤖 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 `@src/notifications/state.ts`:
- Around line 53-61: bumpSessionCount currently does an unlocked
read-modify-write using readState() and writeState(), which can drop concurrent
increments; wrap the increment/write portion in cross-process synchronization by
either (a) acquiring a named lock (e.g., lock file) before calling readState()
and releasing it after writeState(), or (b) implementing a CAS-style retry loop:
readState(), compute next only if lastCountedSessionId differs, attempt atomic
write (or writeState only if persisted state unchanged), and retry a few times
on conflict; ensure you still respect the early-return when sessionId is falsy
or equals lastCountedSessionId and reference the bumpSessionCount, readState,
writeState and lastCountedSessionId symbols when making the change.
In `@tests/claude-code/notifications-referral-invite.test.ts`:
- Around line 35-36: Replace the substring assertions on the notification object
with exact-string assertions: change the two expects that reference n!.title and
n!.body from toContain(...) to toEqual(...) (or toBe(...)) and supply the full
expected title and body literal strings (the current correct user-facing copy)
instead of "$20" and "hivemind invite"; update the test's expected values for
n!.title and n!.body to the exact messages used by the code under test so the
test fails on any text regression.
---
Nitpick comments:
In `@tests/claude-code/notifications-referral-invite.test.ts`:
- Around line 40-41: Replace the loose null-check with a strict payload identity
assertion: call referralInviteRule.evaluate(ctx({ creds: signedIn, sessionCount:
9 })) and assert the returned object matches the expected notification shape
(e.g., toMatchObject({ id: "referral-invite", dedupKey: { v: 1 } })) so the test
verifies both firing and the exact notification payload rather than only
non-null.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9388869a-ed36-4570-8ecb-9d7695ce85d7
📒 Files selected for processing (6)
src/hooks/session-notifications.tssrc/notifications/index.tssrc/notifications/rules/referral-invite.tssrc/notifications/state.tssrc/notifications/types.tstests/claude-code/notifications-referral-invite.test.ts
- referral-invite rule: drop the always-true REFERRAL_NUDGE_ENABLED kill switch (dead branch hurting branch coverage); disabling = unregister the rule. - tests: exact-string assertions on the banner copy (per coding guidelines); add state-counter coverage — writeState/readState round-trip, legacy state (no counter), and markShown preserving the counter fields. - vitest.config: add per-file coverage thresholds for the new rule file.
What
A one-time SessionStart banner nudging signed-in users to invite teammates and earn the referral credit (pairs with the deeplake-api referral program — inviter's org gets $20 when an invited friend signs up).
Cadence
REFERRAL_NUDGE_ENABLEDfor dark-shipping — on, since referral is live on prod.How
rules/referral-invite.ts: pure rule gated oncreds+sessionCount >= 3.notifications-state.json:bumpSessionCountadvances once per session, deduped by session_id so the two parallel SessionStart fires (settings.json + marketplace hooks.json) don't double-count.readState/markShownpreserve the counter fields.sessionCountinto the rule context (rules stay IO-free).Client-only by design: not per-org-cap-aware (that's server state) — worst case it nudges an already-capped referrer once, which is fine for a single banner since "referral enabled" is global.
Tests
Rule cadence + sign-in gating, and
bumpSessionCount(increment / same-id dedup / missing-id).Note: the 2 pre-existing "built artifact" welcome bundle tests fail on clean main too (unrelated — welcome→additionalContext expectation), not touched here.
Follow-up
Needs a plugin version bump / release for users to receive it.
🤖 Generated with Claude Code
Summary by CodeRabbit