Skip to content

[Server] Stop Session::forget() from creating missing segments - #484

Open
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-29-session-forget
Open

[Server] Stop Session::forget() from creating missing segments#484
chr-hertel wants to merge 1 commit into
modelcontextprotocol:mainfrom
chr-hertel:fix/issue-29-session-forget

Conversation

@chr-hertel

Copy link
Copy Markdown
Member

Session::forget() reused set()'s loop body, so deleting a nested key vivified missing intermediate segments: forget('a.b') left a => [] behind, and a non-array intermediate got clobbered into an array. It now returns early when the path doesn't exist.

Regression tests included.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused implementation correctly resolves the mutation bug and includes appropriate regression coverage.

Pull request overview

Prevents Session::forget() from creating or overwriting intermediate path segments when a nested key is absent.

Changes:

  • Return early when an intermediate segment is missing or non-array.
  • Add regression tests for both scenarios.
File summaries
File Description
src/Server/Session/Session.php Safely aborts deletion for invalid intermediate paths.
tests/Unit/Server/Session/SessionTest.php Covers missing and non-array intermediate segments.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

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

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.

2 participants