fix(session): canonical end + idle purge orphan code sweep

[gelbh]

Summary

• Shared endSessionCanonical writes status/endedAt/gameOutcome, clears the code field, and deletes sessionCodes/{code} in one transaction (preserves terminal outcomes like found).
• Idle auto-end uses abandoned outcome; purge raises idle batch throughput and sweeps orphan/ended join codes with a persisted cursor so cron pages advance.
• Finalize game results also runs on abandoned; orphan sweep failures are isolated so stale-session purge still proceeds.

Test plan

• cd functions && node --test test/endSessionCanonical.test.mjs test/autoEndIdleSessions.test.mjs test/orphanSessionCodes.test.mjs
• PR CI unit / emulator / e2e green
• After deploy: oneshot dry-run (later slice) before apply

Summary by CodeRabbit

• Bug Fixes

  • Improved cleanup of expired and abandoned sessions.
  • Removed orphaned session codes that no longer belong to active sessions.
  • Preserved existing terminal outcomes when sessions are finalized concurrently.
  • Ensured game results finalize correctly for abandoned sessions.
  • Improved handling of session codes when a session is already ended.

• Tests

  • Added coverage for session finalization, orphan-code cleanup, and abandoned-session handling.

[coderabbitai]

Warning

Review limit reached

@gelbh, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e1346ea4-3888-43dd-adf0-ca0ed6986785

📥 Commits

Reviewing files that changed from the base of the PR and between f0b4fa4 and a1c6a8b.

📒 Files selected for processing (10)

• functions/handlers/triggers.mjs
• functions/session/autoEndIdleSessions.mjs
• functions/session/endSessionCanonical.mjs
• functions/session/finalizeGameResult.mjs
• functions/session/orphanSessionCodes.mjs
• functions/session/purgeStaleSessions.mjs
• functions/test/autoEndIdleSessions.test.mjs
• functions/test/endSessionCanonical.test.mjs
• functions/test/finalizeGameResult.test.mjs
• functions/test/orphanSessionCodes.test.mjs

Walkthrough

The PR centralizes session termination in a transactional helper, preserves terminal outcomes, expands result finalization for abandoned sessions, adds cursor-based orphan session-code cleanup, and integrates that cleanup and higher idle-session batch limits into the scheduled purge.

Changes

Session lifecycle and cleanup

Layer / File(s)	Summary
Canonical session termination functions/session/endSessionCanonical.mjs, functions/session/autoEndIdleSessions.mjs, functions/test/endSessionCanonical.test.mjs, functions/test/autoEndIdleSessions.test.mjs	Idle sessions use transactional canonical ending with abandoned outcomes, terminal-outcome preservation, session-code deletion, and idempotent handling.
Terminal result finalization functions/session/finalizeGameResult.mjs, functions/test/finalizeGameResult.test.mjs	Finalization now detects newly assigned ended_early or abandoned outcomes and prefers a valid stored endedAt.
Cursor-based orphan code sweep functions/session/orphanSessionCodes.mjs, functions/test/orphanSessionCodes.test.mjs	A paginated sweep identifies missing or ended sessions, deletes orphan codes, and persists its cursor across runs.
Scheduled purge integration functions/handlers/triggers.mjs, functions/session/purgeStaleSessions.mjs	Idle queries use a separate batch limit, orphan cleanup is invoked with captured failures, and purge logs include orphan deletions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ScheduledPurge
  participant OrphanSweep
  participant SessionCodes
  participant Sessions
  participant SystemConfig
  ScheduledPurge->>OrphanSweep: invoke cleanup
  OrphanSweep->>SystemConfig: read cursor
  OrphanSweep->>SessionCodes: query ordered page
  OrphanSweep->>Sessions: load linked sessions
  Sessions-->>OrphanSweep: return session state
  OrphanSweep->>SessionCodes: delete orphan codes
  OrphanSweep->>SystemConfig: write next cursor

Loading

Possibly related PRs

• gelbh/jetlag#18: Related idle-session ending and sessionCodes cleanup changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/lifecycle-end-purge

---

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

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@functions/session/endSessionCanonical.mjs`:
- Around line 22-26: Clear the session’s stale code field in the already-ended
branch of endSessionCanonical by transactionally updating the session document
before returning, while retaining deletion of sessionCodes/{code}. Update
functions/test/endSessionCanonical.test.mjs lines 68-85 to expect this cleanup
write and assert that its payload clears code.

In `@functions/session/orphanSessionCodes.mjs`:
- Around line 88-105: Update the orphan-session processing around withSessions
and the final deletion to batch Firestore operations: collect valid session
document references and load them with db.getAll, then associate the returned
snapshots with each code document, and replace the per-entry delete Promise.all
with a single WriteBatch or BulkWriter operation. Preserve
selectOrphanCodeDocs(withSessions, limit) behavior and delete only the selected
orphan code documents.

In `@functions/session/purgeStaleSessions.mjs`:
- Around line 4-5: Ensure purgeStaleSessions completes within its scheduled
execution window by either configuring an explicit timeoutSeconds on its
onSchedule definition or reducing IDLE_PURGE_BATCH_LIMIT from 200 to a safe
sequential-processing size; preserve the existing orphan-code sweep and
delete-purge behavior.

In `@functions/test/orphanSessionCodes.test.mjs`:
- Around line 111-173: Add a test for sweepOrphanSessionCodes where
cursorState.lastCodeId points beyond the final document and the startAfterId
query returns an empty page, verifying the sweep wraps to the first page and
processes orphan codes there. Use the existing buildSweepDb and cursorState
fixtures, and assert the deleted IDs and returned orphan count.

🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: db2aeadd-8e94-4d1b-af0e-3fa47bc18582

📥 Commits

Reviewing files that changed from the base of the PR and between e1fe678 and f0b4fa4.

📒 Files selected for processing (10)

• functions/handlers/triggers.mjs
• functions/session/autoEndIdleSessions.mjs
• functions/session/endSessionCanonical.mjs
• functions/session/finalizeGameResult.mjs
• functions/session/orphanSessionCodes.mjs
• functions/session/purgeStaleSessions.mjs
• functions/test/autoEndIdleSessions.test.mjs
• functions/test/endSessionCanonical.test.mjs
• functions/test/finalizeGameResult.test.mjs
• functions/test/orphanSessionCodes.test.mjs

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Clear the session’s stale code in the already-ended branch.

This branch deletes sessionCodes/{code} but returns without deleting sessions.code, breaking the canonical end-state invariant.

• functions/session/endSessionCanonical.mjs#L22-L26: transactionally delete the session code field before returning.
• functions/test/endSessionCanonical.test.mjs#L68-L85: expect that cleanup update and assert its payload clears code.

Proposed fix

 if (data.status === "ended" || typeof data.endedAt === "string") {
+  tx.update(sessionRef, { code: FieldValue.delete() });
   if (code) {
     tx.delete(db.collection("sessionCodes").doc(code));
   }
   return;
 }

As per path instructions, review Firestore write safety and error handling.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (data.status === "ended" || typeof data.endedAt === "string") {
	if (code) {
	tx.delete(db.collection("sessionCodes").doc(code));
	}
	return;
	if (data.status === "ended" || typeof data.endedAt === "string") {
	tx.update(sessionRef, { code: FieldValue.delete() });
	if (code) {
	tx.delete(db.collection("sessionCodes").doc(code));
	}
	return;

📍 Affects 2 files

• functions/session/endSessionCanonical.mjs#L22-L26 (this comment)
• functions/test/endSessionCanonical.test.mjs#L68-L85

🤖 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 `@functions/session/endSessionCanonical.mjs` around lines 22 - 26, Clear the
session’s stale code field in the already-ended branch of endSessionCanonical by
transactionally updating the session document before returning, while retaining
deletion of sessionCodes/{code}. Update
functions/test/endSessionCanonical.test.mjs lines 68-85 to expect this cleanup
write and assert that its payload clears code.

Source: Path instructions

[coderabbitai]

🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Batch the per-doc session reads and code deletes.

Each page issues up to limit individual sessions.doc(id).get() calls and up to limit individual codeDoc.ref.delete() calls via Promise.all. Firestore Admin SDK supports batched reads (db.getAll(...)) and batched writes (WriteBatch/BulkWriter), which reduce round trips from O(limit) to O(1) and lower cost/latency, especially now that this runs on every scheduled purge with limit up to 100.

♻️ Example using batched reads

-  const withSessions = await Promise.all(
-    codesSnap.docs.map(async (codeDoc) => {
-      const codeData = codeDoc.data() ?? {};
-      const sessionId =
-        typeof codeData.sessionId === "string" ? codeData.sessionId : null;
-
-      let sessionData = null;
-      if (sessionId) {
-        const sessionSnap = await db.collection("sessions").doc(sessionId).get();
-        sessionData = sessionSnap.exists ? sessionSnap.data() : null;
-      }
-
-      return { codeDoc, codeData, sessionData };
-    }),
-  );
+  const codeEntries = codesSnap.docs.map((codeDoc) => {
+    const codeData = codeDoc.data() ?? {};
+    const sessionId =
+      typeof codeData.sessionId === "string" ? codeData.sessionId : null;
+    return { codeDoc, codeData, sessionId };
+  });
+
+  const sessionRefs = codeEntries
+    .filter((e) => e.sessionId)
+    .map((e) => db.collection("sessions").doc(e.sessionId));
+  const sessionSnaps = sessionRefs.length ? await db.getAll(...sessionRefs) : [];
+  const sessionDataById = new Map(
+    sessionSnaps.filter((s) => s.exists).map((s) => [s.id, s.data()]),
+  );
+
+  const withSessions = codeEntries.map((e) => ({
+    codeDoc: e.codeDoc,
+    codeData: e.codeData,
+    sessionData: e.sessionId ? sessionDataById.get(e.sessionId) ?? null : null,
+  }));

🤖 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 `@functions/session/orphanSessionCodes.mjs` around lines 88 - 105, Update the
orphan-session processing around withSessions and the final deletion to batch
Firestore operations: collect valid session document references and load them
with db.getAll, then associate the returned snapshots with each code document,
and replace the per-entry delete Promise.all with a single WriteBatch or
BulkWriter operation. Preserve selectOrphanCodeDocs(withSessions, limit)
behavior and delete only the selected orphan code documents.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files 'functions/**' | sed -n '1,120p'

echo
echo "== outline purgeStaleSessions.mjs =="
ast-grep outline functions/session/purgeStaleSessions.mjs --view expanded || true

echo
echo "== outline triggers.mjs =="
ast-grep outline functions/handlers/triggers.mjs --view expanded || true

echo
echo "== relevant lines purgeStaleSessions.mjs =="
cat -n functions/session/purgeStaleSessions.mjs | sed -n '1,120p'

echo
echo "== relevant lines triggers.mjs =="
cat -n functions/handlers/triggers.mjs | sed -n '1,260p'

echo
echo "== firebase-functions version =="
cat package.json | jq -r '.dependencies["firebase-functions"], .devDependencies["firebase-functions"]' 2>/dev/null || true

Repository: gelbh/jetlag

Length of output: 14015

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== search for timeoutSeconds and schedules =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'timeoutSeconds|onSchedule|schedule:' functions package.json

echo
echo "== search for scheduled function definitions =="
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' 'pubsub\.schedule|onSchedule\(' functions

Repository: gelbh/jetlag

Length of output: 595

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== firebase-functions package files =="
fd -a 'firebase-functions' . | sed -n '1,50p'

Repository: gelbh/jetlag

Length of output: 189

---

🌐 Web query:

firebase functions v2 scheduler onSchedule default timeoutSeconds when not set documentation

💡 Result:

For Firebase Functions (2nd generation) using the onSchedule handler, the default timeout is 60 seconds when timeoutSeconds is not explicitly set [1][2][3]. While the underlying Google Cloud Functions v2 infrastructure may sometimes have different defaults in other contexts (such as Cloud Run services), the Cloud Functions for Firebase documentation consistently identifies 60 seconds as the default timeout for these deployments [1]. You can override this default by providing a timeoutSeconds value within the options object when calling onSchedule [4][5]: javascript import { onSchedule } from "firebase-functions/v2/scheduler"; export const myScheduledFunction = onSchedule( { schedule: "every day 00:00", timeoutSeconds: 300, // Explicitly set to 5 minutes }, async (event) => { // Function logic here } ); For 2nd-gen scheduled functions, the maximum allowable timeout is 1800 seconds (30 minutes) [5][6][1].

Citations:

• 1: https://deepwiki.com/firebase/firebase-functions/4-function-configuration
• 2: https://docs.cloud.google.com/functions/docs/reference/rpc/google.cloud.functions.v2
• 3: https://cloud.google.com/php/docs/reference/cloud-functions/latest/V2.ServiceConfig
• 4: https://stackoverflow.com/questions/76503936/how-to-increase-memory-for-scheduled-2nd-generation-google-cloud-function-withou
• 5: https://www.rapidevelopers.com/firebase-tutorial/how-to-schedule-background-functions-in-firebase
• 6: https://firebase.google.com/docs/reference/functions/2nd-gen/node/firebase-functions.globaloptions

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== autoEndIdleSessions.mjs =="
cat -n functions/session/autoEndIdleSessions.mjs | sed -n '1,260p'

echo
echo "== orphanSessionCodes.mjs =="
cat -n functions/session/orphanSessionCodes.mjs | sed -n '1,260p'

Repository: gelbh/jetlag

Length of output: 6068

---

Idle purge batch is too large for the scheduled function’s default 60s timeout.

IDLE_PURGE_BATCH_LIMIT jumps to 200, but purgeStaleSessions still processes each idle session sequentially and now also runs the orphan-code sweep and delete purge in the same invocation. Since onSchedule has no explicit timeoutSeconds, this function inherits the 60s default, so the run can be cut off mid-batch. Set an explicit timeout or reduce the batch size.

🤖 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 `@functions/session/purgeStaleSessions.mjs` around lines 4 - 5, Ensure
purgeStaleSessions completes within its scheduled execution window by either
configuring an explicit timeoutSeconds on its onSchedule definition or reducing
IDLE_PURGE_BATCH_LIMIT from 200 to a safe sequential-processing size; preserve
the existing orphan-code sweep and delete-purge behavior.

[coderabbitai]

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the empty-page wraparound branch.

Tests cover the full-page and partial-page-at-end cases, but not the scenario where a page fetched with startAfterId returns empty (cursor past the real end), which should wrap back to the first page (orphanSessionCodes.mjs lines 84-86). Consider adding a test where the stored cursor points past the last document.

🤖 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 `@functions/test/orphanSessionCodes.test.mjs` around lines 111 - 173, Add a
test for sweepOrphanSessionCodes where cursorState.lastCodeId points beyond the
final document and the startAfterId query returns an empty page, verifying the
sweep wraps to the first page and processes orphan codes there. Use the existing
buildSweepDb and cursorState fixtures, and assert the deleted IDs and returned
orphan count.