fix(sdk): respect the 30-minute session expiry in getSessionId

[r69shabh]

Fixes #694

What

getSessionId() reads did_session from sessionStorage without checking did_session_timestamp. The tracker rotates sessions after 30 minutes of inactivity, so between expiry and the tracker's next visit the SDK hands back a session id that is no longer the current one. getTrackingIds() and getTrackingParams() inherit the problem since they delegate to it.

Why it matters

getTrackingParams() exists to carry the ids to another origin for cross-domain tracking. If the session has actually expired, propagating the stale id links events to a session the tracker has already closed — anyone keying server-side joins on these ids gets subtly wrong data.

How

getSessionId() now reads the timestamp alongside the id and returns null if it's missing or older than 30 minutes — the same rule the tracker uses (sessionAge < 30 * 60 * 1000 in getOrCreateSessionId()). The window is a named constant with a comment pointing at the tracker logic. URL-param priority is untouched, and getTrackingIds/getTrackingParams are unchanged — they get the behavior for free.

Tests: the existing did_session setups now write a fresh timestamp, plus new cases for no-timestamp → null, older than 30 minutes → null, and exactly at the boundary → expired (matching the tracker's strict < check).

Tests

109 passed (sdk e2e suite, chromium)
7 getSessionId-specific tests passing
tsc --noEmit: 0 errors
bun run test (monorepo): 27/27 tasks passed

---

AI disclosure: I used Claude (via Qoder) for research and writing. I found the mismatch myself while comparing the tracker's expiry logic against the SDK helper, reproduced it in the e2e suite, and sketched the fix — Claude helped complete the implementation and fill in the test cases from that. I reviewed all the generated code and ran type-check and the full e2e suite locally; the pre-push hook ran the monorepo suite clean.

---

Summary by cubic

getSessionId() now respects the tracker's 30-minute session expiry when the tracker isn't running, returning null for missing or expired timestamps (with the same strict < boundary the tracker uses), and returning the stored id as-is while the tracker is on the page.

• getTrackingIds() and getTrackingParams() inherit the fix; URL params still take priority.
• Updates e2e suite selectors to match current dashboard UI copy, including the business context link label case.
• Bumps the ClickHouse revenue integration test timeout for the slow query paths.

^{Written for commit 06c9536. Summary will update on new commits.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 1cb1672a-4ed9-4c04-a0f4-6ab756714f91

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[vercel]

@r69shabh is attempting to deploy a commit to the Databuddy OSS Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Confidence Score: 5/5

The current head appears safe to merge, with no outstanding correctness, security, or repository-rule issues identified.

Summary

This PR aligns the SDK’s public session-ID helpers with the tracker’s 30-minute inactivity window.

• Validates the stored session timestamp before returning did_session.
• Treats missing, invalid, and expired timestamps as having no active session.
• Preserves URL-parameter priority for cross-domain tracking.
• Adds expiry, missing-timestamp, boundary, and fresh-session test coverage.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[getSessionId called] --> B{Session ID in URL parameters?}
    B -- Yes --> C[Return URL session ID]
    B -- No --> D[Read did_session from sessionStorage]
    D --> E{Stored ID exists?}
    E -- No --> F[Return null]
    E -- Yes --> G[Read did_session_timestamp]
    G --> H{Timestamp valid and age under 30 minutes?}
    H -- No --> F
    H -- Yes --> I[Return stored session ID]

Loading

_{Reviews (2) · Last reviewed commit: "fix(sdk): respect the 30-minute session ..."}

[izadoesdev]

@greptile review

Please review current head 5e753a2 for correctness and security as part of the requested review of all open PRs. Recheck earlier findings against this head and distinguish unresolved issues from already-fixed feedback.

[izadoesdev]

Verified against the tracker rather than the description, since getting this wrong would make getSessionId() return null for every real visitor.

packages/tracker/src/index.ts:315-316 and packages/tracker/src/core/tracker.ts:286-287 write did_session and did_session_timestamp together on every session write, so the timestamp this reads is always present for a tracker-created session. The boundary matches exactly: the tracker rotates on sessionAge >= 30 * 60 * 1000 (index.ts:313) and keeps on sessionAge < 30 * 60 * 1000 (core/tracker.ts:276), so treating exactly 30 minutes as expired is correct, and the boundary test asserts the right side of it.

The did_session without a timestamp case is also safe to treat as expired: core/tracker.ts:271-282 removes both keys when the timestamp is missing or stale, so that pairing is not a state the tracker produces.

One non-blocking note: the 30-minute window is now a constant in both packages/tracker and packages/sdk with only a comment tying them together. That will drift. A shared constant, or a test that asserts the two agree, would be the durable version.

The three red checks are Vercel – dashboard, – databuddy-status and – documentation, which are the fork-preview authorization step rather than builds; staging has no required status checks.

[izadoesdev]

Requesting changes after a closer look at how the tracker maintains did_session_timestamp, which my earlier approval missed.

The tracker only writes that timestamp on page load (packages/tracker/src/core/tracker.ts, session init) and on back/forward cache restore (packages/tracker/src/index.ts, the pageshow handler). In-app navigation in a single-page app never refreshes it, yet the tracker keeps sending the same did_session for the life of the page. With this change, 30 minutes after an SPA loads, getSessionId() returns null while events still carry that session id, so anything that reads getTrackingIds() at that point (checkout metadata for revenue attribution, for example) loses the session.

Could you apply the expiry only when no tracker is running on the page? If isTrackerAvailable() is true, return the stored did_session as is. Please add a test for that case too: tracker present plus a 31-minute-old timestamp still returns the id.

[Copilot]

Copilot review overview

🟡 Changes recommended

Make the boundary test deterministic by freezing Date.now().

Review effort: Lite
Findings: 1

Open (1)

• Freeze Date.now to deterministically test the exact 30-minute boundary · New

What changed in this PR

Updates SDK session handling to reject missing or expired session IDs after 30 minutes.

Changes:

• Adds timestamp-based session expiry validation.
• Expands tests for fresh, missing, expired, and boundary timestamps.
• Adds an SDK patch changeset.

File	Summary
packages/​sdk/​tests/​sdk-functions.spec.ts	Adds session expiry coverage; the exact-boundary test should freeze Date.now() for deterministic equality.
packages/​sdk/​src/​core/​tracker.ts	Validates stored session age before returning the session ID.
.changeset/​sdk-session-id-expiry.md	Documents the SDK patch release.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[r69shabh]

Sorry for the late commit, was occupied over the week.

Addressed both pieces of feedback:

@izadoesdev's change request — the expiry check now only applies when no tracker is running on the page. While rebasing onto latest staging, I found it now delegates getSessionId()/getAnonymousId() to the live tracker instance first (getTracker()?.getTrackingIds?.()), which already resolves your SPA concern structurally: with a current tracker, the SDK returns the tracker's own live ids and never consults the timestamp. On top of that:

• the 30-minute check remains only on the tracker-absent storage fallback (the cross-domain getTrackingParams() case from getSessionId() returns session ids the tracker has already expired #694),
• plus an explicit isTrackerAvailable() early-return so trackers too old to expose getTrackingIds also return the stored id as-is instead of nulling it at 30 minutes.

New tests: tracker present + 31-minute-old timestamp returns the id, for both the modern delegation path and the legacy no-getTrackingIds path.

Copilot's boundary determinism — the exact-30-minute test now installs a fixed clock (page.clock.install({ time })) so Date.now() can't drift between setup and assertion.

Verified: full sdk-functions.spec.ts on chromium (43 passed), tsc --noEmit clean, Biome clean, and the monorepo pre-push suite is 32/32 green.

One open thread from your earlier note: the 30-minute window is still a duplicated constant in tracker and sdk tied together only by a comment. Happy to do the shared-constant (or agreement-test) version as a follow-up if you want it in this PR.