feat: add session tracking

[gunoooo]

Summary

• Add SessionService that tracks SESSION_START events based on app lifecycle (ProcessLifecycleOwner) with configurable timeout
• Add sessionTimeoutMs to ClixConfig (default: 30s, minimum: 5s)
• Set pendingMessageId on push notification tap so the subsequent SESSION_START event includes the messageId

Changes

• New: clix/src/main/kotlin/so/clix/services/SessionService.kt — session timeout logic, ProcessLifecycleOwner observer, SessionEvent enum
• Modified: clix/src/main/kotlin/so/clix/core/Clix.kt — initialize and start SessionService
• Modified: clix/src/main/kotlin/so/clix/core/ClixConfig.kt — add sessionTimeoutMs property
• Modified: clix/src/main/kotlin/so/clix/notification/NotificationTappedActivity.kt — call setPendingMessageId on notification tap

Spec

• https://www.notion.so/greyboxhq/Tech-Clix-Session-3031f02665f180dfa04bc91a836f0889

Summary by CodeRabbit

• New Features
  • Added session management to the Clix SDK with a configurable session timeout (default: 30 seconds).
  • Sessions automatically initialize, persist last activity, and emit lifecycle events to improve analytics.
  • Notification taps now attach a pending message identifier so tapped messages are linked to the current or next session.

[coderabbitai]

Walkthrough

Adds an internal SessionService and a new ClixConfig.sessionTimeoutMs; Clix.initialize now constructs and starts SessionService and stores it on Clix. Notification tap handling sets a pending messageId on the sessionService. SessionService observes process lifecycle, persists last-activity timestamps, applies timeout logic, and emits SESSION_START events.

Changes

Cohort / File(s)	Summary
Core Session Management clix/src/main/kotlin/so/clix/services/SessionService.kt	New internal SessionService and SessionEvent enum: lifecycle-aware session tracking via ProcessLifecycleOwner, persisted last-activity timestamp, effective timeout (min 5000ms), pendingMessageId handling, session continuation/new-session logic, and async SESSION_START event dispatch with error handling.
SDK Initialization / Config clix/src/main/kotlin/so/clix/core/Clix.kt, clix/src/main/kotlin/so/clix/core/ClixConfig.kt	Added internal var sessionService: SessionService? to Clix; Clix.initialize now constructs (SessionService(storageService, eventService, config.sessionTimeoutMs)) and starts the service. ClixConfig adds val sessionTimeoutMs: Int = 30_000 with KDoc.
Notification Integration clix/src/main/kotlin/so/clix/notification/NotificationTappedActivity.kt	On notification tap, extracts messageId and calls Clix.sessionService?.setPendingMessageId(messageId) before delegating to existing notification handling.

Possibly related PRs

• fix: handle push received when app process is killed #12: Adjusts Clix initialization and persistence logic—overlaps with this PR's wiring of SessionService into initialization and storage usage.
• feat: refine push notification handling and lifecycle support #14: Modifies Clix lifecycle and notification flows—overlaps with process-lifecycle integration and notification tap handling changes.

Suggested reviewers

• nyanxyz
• JeongwooYoo

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: add session tracking' clearly summarizes the main change: introducing session tracking functionality via SessionService with lifecycle management and event tracking.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch CLIX-115

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

clix/src/main/kotlin/so/clix/services/SessionService.kt (3)

53-68: Direct coupling to Clix.coroutineScope reduces testability.

startNewSession reaches into the Clix singleton for its coroutine scope. Consider injecting the CoroutineScope as a constructor parameter (alongside storageService and eventService) so the class remains independently testable without needing the full SDK initialized.

Suggested change

 internal class SessionService(
     private val storageService: StorageService,
     private val eventService: EventService,
     sessionTimeoutMs: Int,
+    private val coroutineScope: CoroutineScope,
 ) : DefaultLifecycleObserver {
     ...
     private fun startNewSession() {
         val messageId = pendingMessageId
         pendingMessageId = null
         updateLastActivity()
 
-        Clix.coroutineScope.launch {
+        coroutineScope.launch {

Based on learnings: "Do not use service locators or singletons (unless absolutely necessary)."

---

22-22: pendingMessageId read-then-clear is not atomic.

setPendingMessageId (Line 49) can be called from the notification-tap path while startNewSession (Lines 54-55) reads and clears it on the main thread via lifecycle. Since both should run on the main thread this is low-risk today, but @Volatile alone doesn't guarantee atomicity of the read-then-null sequence. If this ever gets called off the main thread the value could be lost silently.

If you keep @Volatile, consider using getAndSet-style access via AtomicReference<String?> for a small safety improvement, or just add a comment documenting the main-thread assumption.

Also applies to: 49-51, 53-55

---

10-12: Single-member enum — consider whether an enum is warranted.

SessionEvent only has SESSION_START. If you plan to add more session events (e.g., SESSION_END) this is fine. Otherwise a simple const val would suffice.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e9c3fd758

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

clix/src/main/kotlin/so/clix/core/Clix.kt (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Pipeline failure: ktfmt formatting check failed for this file.

The CI reports this file is not properly formatted. Please run ktfmt to fix formatting before merging.

🤖 Fix all issues with AI agents

In `@clix/src/main/kotlin/so/clix/core/ClixConfig.kt`:
- Line 22: Add a `@property` entry for sessionTimeoutMs to the ClixConfig KDoc
explaining that sessionTimeoutMs is the session timeout in milliseconds used to
end idle sessions, state the default value (30_000 ms) and document the minimum
enforced value as implemented by ClixConfig (refer to the validation or init
logic that enforces the minimum) so readers know the lower bound and default.

In `@clix/src/main/kotlin/so/clix/services/SessionService.kt`:
- Line 1: The file with the package declaration "package so.clix.services"
failed ktfmt formatting; run the project's ktfmt formatter (e.g., ktfmt -w on
the file or the project's gradle task such as ./gradlew ktfmtFormat) to reformat
the file, verify there are no ktfmt violations, and commit the formatted changes
so the CI formatting check passes.
- Around line 29-54: The session continuation/check logic in start() and
onStart() is duplicated and can cause duplicate SESSION_START when addObserver
synchronously triggers onStart(); extract the shared logic into a single private
helper (e.g., ensureSessionOrStart or checkOrStartSession) that performs the
LAST_ACTIVITY_KEY read, elapsed check, updateLastActivity(), and
startNewSession(); then modify start() to only call
ProcessLifecycleOwner.get().lifecycle.addObserver(this) (no session check) and
call the new helper from onStart(), removing the duplicated block from start(),
ensuring startNewSession() can only be invoked once per lifecycle transition.

🧹 Nitpick comments (4)

clix/src/main/kotlin/so/clix/services/SessionService.kt (2)

69-69: Clix.coroutineScope creates tight coupling to the singleton.

Referencing Clix.coroutineScope directly inside SessionService violates the dependency-injection principle and makes this class harder to test. Inject the CoroutineScope as a constructor parameter instead.

Proposed fix

 internal class SessionService(
     private val storageService: StorageService,
     private val eventService: EventService,
     sessionTimeoutMs: Int,
+    private val coroutineScope: CoroutineScope,
 ) : DefaultLifecycleObserver {

Then replace Clix.coroutineScope.launch with coroutineScope.launch.

Based on learnings: "Do not use service locators or singletons (unless absolutely necessary)".

---

26-27: @Volatile alone is insufficient if pendingMessageId is read/cleared from a coroutine while set from the main thread.

setPendingMessageId is called from the UI thread (NotificationTappedActivity), while startNewSession reads and clears it potentially from the same main thread (lifecycle callback). In this specific flow both happen on the main thread, so @Volatile is sufficient. However, if the coroutine on line 69 or any future caller invokes this from a background thread, the read-then-clear on lines 65-66 is not atomic. Consider using AtomicReference with getAndSet(null) to be safe, or document the threading contract.

clix/src/main/kotlin/so/clix/core/Clix.kt (2)

86-87: sessionService?.start() — safe-call is unnecessary immediately after assignment.

Line 86 assigns a non-null SessionService instance, so line 87's ?. is redundant. Use sessionService!!.start() or, better, a local val:

Proposed fix

-                sessionService = SessionService(storageService, eventService, config.sessionTimeoutMs)
-                sessionService?.start()
+                val session = SessionService(storageService, eventService, config.sessionTimeoutMs)
+                sessionService = session
+                session.start()

---

86-87: SessionService.start() registers a ProcessLifecycleOwner observer, which requires the main thread.

The initialize method is documented to be called from Application.onCreate() (main thread), but there's no runtime assertion. If a caller ever invokes initialize from a background thread, addObserver could throw or behave unexpectedly. Consider adding a main-thread check or documenting this requirement explicitly.

[pitzcarraldo]

lgtm. please just check the comments left by review bots.