Skip to content

fix: align state prefixes with State constants - #1650

Open
hemasekhar-p wants to merge 1 commit into
google:mainfrom
hemasekhar-p:issue1645-fix
Open

hemasekhar-p wants to merge 1 commit into
google:mainfrom
hemasekhar-p:issue1645-fix

Conversation

@hemasekhar-p

Copy link
Copy Markdown
Contributor

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

Problem:
In FirestoreSessionService.appendEvent, state key routing checked for non-canonical string prefixes (app and user) rather than ADK's standard prefix constants State.APP_PREFIX. Consequently:

Mutations destined for shared application state (app:) or user state (user:) fell through into individual session state rather than being routed to their dedicated Firestore collections (app-state and user-state).
Ephemeral keys matching State.TEMP_PREFIX ("temp:") were not skipped and were unintentionally persisted to durable session state.
The deletion sentinel State.REMOVED was treated as a normal value rather than executing FieldValue.delete() in Firestore or session.state().remove(key) in session memory.

Solution:
Updated FirestoreSessionService.appendEvent to route using State.APP_PREFIX and State.USER_PREFIX, stripping the prefix before saving to the respective Firestore document.
Added an explicit check for State.TEMP_PREFIX to ignore ephemeral state during persistence.
Mapped State.REMOVED to FieldValue.delete() for scoped Firestore mutations and invoked session.state().remove(key) for session state.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Manual End-to-End (E2E) Tests:

Validated the issue before and after fix manually in local.

Checklist

  • I have read the CONTRIBUTING.md document.
  • My pull request contains a single commit.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant