Skip to content

fix(core): publish in-memory session for CLI SessionAccess#206

Merged
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/cli-session-access-before-start
Jul 22, 2026
Merged

fix(core): publish in-memory session for CLI SessionAccess#206
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/cli-session-access-before-start

Conversation

@TDannhauer

Copy link
Copy Markdown
Contributor

Summary

  • Fix fatal SessionAccessor::current() during CLI appInit (e.g. horde-alarms) after the SessionAccess refactor.
  • Publish an in-memory HordeSession for setup(false) without calling session_start().
  • Guard Nlsconfig session caching with hasCurrent().

Motivation

CLI scripts deliberately skip PHP session files (Horde_Session::setup(false)). After consumers switched to SessionAccess, that left the accessor empty, so language setup and notification handler attach crashed with LogicException.

Changes

  • SessionLifecycle::publishInMemory() — publish accessor/injector binding; keep isActive() false (no $_SESSION mirror).
  • Horde_Session::setup(false) calls publishInMemory() when a lifecycle is wired.
  • Horde_Registry_Nlsconfig skips session cache when no current session is published.
  • Unit coverage for both paths.

Test plan

  • vendor/bin/horde-alarms completes (exit 0) on a Horde 6 deployment
  • SessionLifecycleTest::testPublishInMemory…
  • NlsconfigTest cases without established session
  • Spot-check a normal web login still starts a real PHP session

@TDannhauer
TDannhauer requested a review from ralflang July 22, 2026 16:10

@ralflang ralflang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As a consolidated PR I cannot accept this. It's important to fix any CLI breakage but not by making the session mechanism some kind of in memory value bag. The point of session is carrying information and mostly identity and grants between request lifecycles. CLI scripts don't really need this and if they depend on sessions for architecture / DI reasons we should not make this part of our session design but fix it in the consumers.

@ralflang

ralflang commented Jul 22, 2026

Copy link
Copy Markdown
Member

See also #207

I re-worked your design a bit. We don't need to introduce a third state where we did not officially start a session but kind of have one. Horde_Session_Null now behaves like a regular legacy Horde_Session shim and interacts with the modern framework.

@ralflang
ralflang force-pushed the fix/cli-session-access-before-start branch from 18d5b17 to 0b90aff Compare July 22, 2026 17:28
…n_Null

Also route CLI appInit through session_control none
Guard CLI and session_control=none scenarios of the legacy registry against code paths using SessionAccess with no session

Co-authored-by: Torben Dannhauer <torben@dannhauer.de>
@ralflang
ralflang force-pushed the fix/cli-session-access-before-start branch from 0b90aff to bffa341 Compare July 22, 2026 17:36
@ralflang
ralflang merged commit f457999 into FRAMEWORK_6_0 Jul 22, 2026
1 check failed
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