Skip to content

fix: defensive null guards on Memory.sessionIds#684

Merged
rohitg00 merged 2 commits into
mainfrom
fix/null-session-ids-defensive
May 27, 2026
Merged

fix: defensive null guards on Memory.sessionIds#684
rohitg00 merged 2 commits into
mainfrom
fix/null-session-ids-defensive

Conversation

@rohitg00
Copy link
Copy Markdown
Owner

@rohitg00 rohitg00 commented May 27, 2026

Bug-fix-triage pass: five callsites assumed Memory.sessionIds is always string[], which the type says — but JSONL imports + older on-disk records can deliver undefined. Touching memory.sessionIds[0] would then throw TypeError: Cannot read properties of undefined.

Read-side (now memory.sessionIds?.[0] ?? "memory"):

  • src/index.ts:485 — BM25 rebuild loop
  • src/state/memory-utils.ts:14memoryToObservation helper
  • src/functions/remember.ts:145vectorIndexAddGuarded call
  • src/functions/search.ts:264 — rebuild loop

Import side (src/functions/export-import.ts): normalise to [] before persisting, so downstream code never has to defend against the missing-field shape.

Full suite locally: 1227 pass, 1 pre-existing integration-test failure (needs running server, unrelated).

Summary by CodeRabbit

  • Bug Fixes
    • Improved robustness of memory import so records with missing or malformed session info are handled safely.
    • Prevented index and search failures by defaulting missing session identifiers, ensuring memories remain indexed and retrievable.
    • Made memory-to-observation and backfill flows tolerant of absent session data to avoid runtime errors.

Review Change Stack

…paths

Memory.sessionIds is declared as string[] but JSONL imports + older on-disk records can carry undefined for it. Four read-side callsites accessed memory.sessionIds[0] directly, which would throw when sessionIds is missing. Switched to optional-chaining (memory.sessionIds?.[0] ?? "memory") in: src/index.ts (BM25 rebuild), src/state/memory-utils.ts (memoryToObservation), src/functions/remember.ts (vector add), src/functions/search.ts (rebuild loop). Import side in src/functions/export-import.ts now normalises Memory.sessionIds to [] when it arrives non-array, so downstream readers can rely on the field being present after import.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentmemory Ready Ready Preview, Comment May 27, 2026 5:32pm

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 45b1a395-73de-4fb5-b522-c3d3ca3796b2

📥 Commits

Reviewing files that changed from the base of the PR and between 029a7c6 and 5fde5cc.

📒 Files selected for processing (1)
  • src/functions/export-import.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/functions/export-import.ts

📝 Walkthrough

Walkthrough

This PR adds defensive handling for missing or non-array sessionIds in Memory objects. On import, invalid sessionIds are normalized to an empty array. Downstream functions in remember, search, index, and memory-utils now safely access sessionIds[0] using optional chaining with a fallback to "memory".

Changes

SessionIds resilience

Layer / File(s) Summary
Import normalization of sessionIds
src/functions/export-import.ts
The import loop defensively normalizes each imported Memory to ensure sessionIds is an array, replacing missing or non-array values with [] before persistence.
Safe sessionId access in consuming functions
src/functions/remember.ts, src/functions/search.ts, src/index.ts, src/state/memory-utils.ts
Vector indexing in remember and search rebuild, BM25 legacy backfill, and memory-to-observation conversion now safely derive sessionId via optional chaining (sessionIds?.[0]) with fallback to "memory", preventing runtime errors when sessionIds is absent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A memory without a session?
No panic, just discretion!
We normalize imports with care,
Access safely everywhere,
With optionals and defaults fair. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding defensive null guards to safely access Memory.sessionIds across multiple locations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/null-session-ids-defensive

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/functions/export-import.ts`:
- Around line 378-381: Remove the explanatory "WHAT" comment block above the
normalization of the sessionIds field (the three-line comment describing that
imported JSONL may carry malformed Memory entries and normalising to an empty
array); delete those lines and, if you feel a comment is still necessary,
replace them with a short WHY note (e.g., "preserve compatibility with
older/hand-edited exports") but do not restate what the code does—locate the
code that normalises the sessionIds field and remove the comment there.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6500ca8-2d88-4d36-905f-b752b5ad2f9d

📥 Commits

Reviewing files that changed from the base of the PR and between 4dd1dde and 029a7c6.

📒 Files selected for processing (5)
  • src/functions/export-import.ts
  • src/functions/remember.ts
  • src/functions/search.ts
  • src/index.ts
  • src/state/memory-utils.ts

Comment thread src/functions/export-import.ts Outdated
@rohitg00 rohitg00 merged commit e39d962 into main May 27, 2026
7 checks passed
@rohitg00 rohitg00 deleted the fix/null-session-ids-defensive branch May 27, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant