FirestoreSessionService ignores app: / user: state prefixes, so app and user state are never shared across sessions #1645

Activity

1. hemasekhar-p

   self-assigned this

   on Oct 12, 2026
2. hemasekhar-p

   added the

   Bug

   issue type on Oct 12, 2026
3. hemasekhar-p commented on Oct 12, 2026

   

   hemasekhar-p

   on Oct 12, 2026

   Contributor

   More actions

   Hi @innoprej, thank you for reporting this issue. I have been able to reproduce this behavior using your inputs and have provided a fix in this PR. I hope this will resolves your problem?
4. hemasekhar-p

   added

   waiting on reporterWaiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.

   on Oct 12, 2026
5. innoprej commented on Oct 12, 2026

   

   innoprej

   on Oct 12, 2026

   ContributorAuthor

   More actions

   Thanks for the quick fix, @hemasekhar-p!

   I checked out #1650 (1329546d) and ran FirestoreSessionServiceTest. The write side looks right to me: app:/user: keys now go to the app-state/user-state documents, temp: keys in the delta are skipped, and State.REMOVED deletes the key (FieldValue.delete() in the shared documents). Your four new or updated appendEvent tests pass on the PR and fail on main (189d463a).

   It doesn't resolve this issue yet, though, because nothing reads those documents back. getSession, createSession and listSessions never look at app-state/user-state (getSession builds the state from the session document only, listSessions returns an empty state), so the steps in the issue still end with user:name missing in session B. And with #1650 alone, an event that only sets app:/user: keys no longer updates the session document's state. So if a tool sets user:name and nothing later in that turn changes a session key, the next getSession of the same session (the runner's next turn) doesn't return it. On main that case worked, because the key fell through to the session state.

   What I ran, with mocked Firestore in the same test class (no emulator):

   Behaves like InMemorySessionService?	main	#1650
   Session B, a new session and listSessions see app:theme / user:name stored in the shared documents	no	no
   Session A, loaded again after a tool set user:name (no other state change in that turn), still has it	yes	no

   To fix the read side, getSession, createSession and listSessions would need to merge the two documents back with the prefixes, like InMemorySessionService.mergeWithGlobalState (here) or _merge_state in the Python Firestore service (here). appendEvent should probably also apply app:/user: delta values to the in-memory session.state(), as BaseSessionService.appendEvent does. #1650 now only sends them to the shared documents, so a value that arrives only through EventActions.stateDelta() is no longer visible later in the same invocation.

   One smaller gap the table doesn't cover: Context.state() writes through to session.state() (Context), and appendEvent still writes the whole map with update("state", session.state()) (here). So temp: keys (and app:/user: keys) set through the context state end up in the session document whenever that event or a later one in the same invocation changes a session key. The Python Firestore service keeps the two apart: it applies the delta to the in-memory session.state (here), as suggested above, but filters these prefixes out before it writes the session document (here). InMemorySessionService keeps them too, since it stores the runner's session object, so this may be better handled separately.

   Would you like to add the read side to #1650, or should I send a follow-up PR after it's merged? Since #1650 says Closes: #1645, could this issue stay open until the read side lands? I'm happy to share the mocked tests I used.
6. hemasekhar-p commented on Oct 14, 2026

   

   hemasekhar-p

   on Oct 14, 2026

   Contributor

   More actions

   @innoprej, thank you for your feedback, i have addressed the comments and updated the PR. hope the updated PR will resolve complete issue?
7. hemasekhar-p

   added

   waiting on reporterWaiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.

   and removed

   waiting on reporterWaiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.Waiting for reaction by reporter. Failing that, maintainers will eventually closed it as stale.

   on Oct 14, 2026
8. innoprej commented on Oct 14, 2026

   

   innoprej

   on Oct 14, 2026

   ContributorAuthor

   More actions

   Thanks for the update, @hemasekhar-p! I ran the mocked tests in FirestoreSessionServiceTest and FirestoreDatabaseRunnerTest against 6b460465 (no emulator). The read side works now: getSession, createSession and listSessions return the shared app:/user: values, a session reloaded after a turn that only set user: keys keeps them, and current shared values override older copies in the session document. The existing service and runner tests still pass (34 + 4), and the four points listed in #1645 look fixed to me.

   Two things remain. First, map values in the shared documents are merged rather than replaced; before #1650, app:/user: values were written as part of the whole session state field. The set(..., SetOptions.merge()) calls (here) send a mask of nested paths: with the real Java client (nothing sent to a server), user:prefs = {lang: ko} gets the mask [prefs.lang], and fields outside the mask are left unchanged (update_mask). So, by that rule, if user:prefs was {lang, theme} before, the next read would still return theme, while InMemorySessionService replaces the value. The Python service writes these documents whole for this reason (here). A top-level mask avoids this: SetOptions.mergeFieldPaths with FieldPath.of(key) for each key gives [prefs] and still accepts FieldValue.delete() (mergeFields(String...) reads a key like settings.v2 as a nested path and fails), so the change looks small.

   Second, when an event changes a session key, appendEvent writes all of session.state(), which now includes the merged app:/user: values, to the session document (here). So a shared key deleted in another session can come back:

   1. Session A sets app:theme and user:name.
   2. Session B is created (it gets both values) and sets counter. B's session document now stores {app:theme, counter, user:name}.
   3. Session A removes both keys with State.REMOVED. The shared documents are now empty, and a new session sees neither key.
   4. Reloading session B still returns both old values.

   InMemorySessionService gives the same result here (even when B's event changes no state), because it stores the runner's session object with the merged values (here). Python filters these prefixes out before writing the session document (here). In a local copy, that filter fixed these steps and the temp: case from my last comment, and the existing tests still passed. On its own, though, it also drops app:/user: values that exist only in a session document on that session's next state write: data saved by the old code, and initial state passed to createSession, which Python splits out (here).

   The first point is Firestore-specific and only touches the two set calls, so could it go into #1650? The second matches InMemorySessionService, so I'm fine with #1650 closing #1645 and tracking it in a follow-up issue, which I can open with the mocked tests.