Skip to content

fix(session): initialize $data property with default empty array#274

Open
woprrr wants to merge 1 commit intomodelcontextprotocol:mainfrom
woprrr:fix/session-data-initialization
Open

fix(session): initialize $data property with default empty array#274
woprrr wants to merge 1 commit intomodelcontextprotocol:mainfrom
woprrr:fix/session-data-initialization

Conversation

@woprrr
Copy link
Copy Markdown

@woprrr woprrr commented Apr 2, 2026

Summary

  • Initialize Session::$data with = [] to prevent TypeError when the property is accessed before initialization on a newly created session
  • Replace isset($this->data) check in readData() with an explicit $loaded flag to preserve lazy loading from the session store
  • Set $loaded = true in clear() and hydrate() to maintain consistent state

Fixes #273

Test plan

  • All 28 existing SessionTest tests pass
  • PHPStan level 6 with no errors
  • PHP-CS-Fixer with no violations
  • testSaveBeforeReadInitializesData confirms save() on a fresh session works
  • testSessionLoadsDataFromStoreOnConstruction confirms lazy loading still works

Prevents TypeError when $data is accessed before initialization on a
newly created session. Replaces the isset($this->data) check in
readData() with an explicit $loaded flag to preserve lazy loading
from the session store.

Fixes modelcontextprotocol#273
@woprrr woprrr force-pushed the fix/session-data-initialization branch from 9e2023d to 1c5398b Compare April 19, 2026 08:34
@woprrr
Copy link
Copy Markdown
Author

woprrr commented Apr 19, 2026

Rebased on top of current main — no conflicts.

The qa job failure is unrelated to this PR: php-cs-fixer reports 13 files, none of which are touched here (Session.php and SessionTest.php both pass locally). Looks like a CI-wide issue with the fixer rules version (3.95.1 in CI vs 3.94.2 in composer.lock).

@pgrimaud
Copy link
Copy Markdown

Thank for this PR @woprrr 😄
Could you fix the CS? 🙏

Thanks

@woprrr
Copy link
Copy Markdown
Author

woprrr commented Apr 20, 2026

@pgrimaud it's impossible because it's not linked to my changes it from dev branch.

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.

Session::save() crashes when $data property is never initialized

2 participants