Skip to content

fix(adk): session query/manage APIs accept an explicit user_id - #108

Closed
shoom1 wants to merge 1 commit into
developfrom
fix/adk-session-user-scope
Closed

fix(adk): session query/manage APIs accept an explicit user_id#108
shoom1 wants to merge 1 commit into
developfrom
fix/adk-session-user-scope

Conversation

@shoom1

@shoom1 shoom1 commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Review finding 5, countered

The finding is technically correct: process() accepts arbitrary user_id (and job-resume threads record.user_id), while session_exists / list_sessions / delete_session / recent_messages hard-coded settings.default_user — sessions created for a non-default user were unreachable through the framework API.

But the proposed fix (a required typed (app_name, user_id, session_id) identity across all processing and session methods) is over-engineering here: ADK's session service already namespaces by exactly that tuple internally, and the shipped CLI is single-user — message_processor passes settings.default_user everywhere, so no current caller can hit the mismatch. The LangGraph thread_id half of the finding targets a backend slated for retirement and is deliberately not addressed.

Change

Keyword-only user_id: str | None = None (defaulting to settings.default_user) on the four ADK session methods. Existing callers are untouched; multi-user library consumers can now manage the sessions they create.

Tests

TDD: seeded sessions under two users against a real sqlite DatabaseSessionService; asserts default scope stays unchanged, explicit scope reaches the other user's session through all four APIs, and cross-user operations don't leak. Full offline suite: 1982 passed.

https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv

Review finding 5 (countered): process() accepts arbitrary user_id and the
job-resume path threads record.user_id, but session_exists/list_sessions/
delete_session/recent_messages hard-coded settings.default_user — sessions
created for another user were unreachable through the framework API.

Add keyword-only user_id (default: settings.default_user) to the four
methods. Deliberately NOT the reviewer's typed (app_name, user_id,
session_id) identity: ADK's session service already namespaces by exactly
that tuple, the shipped CLI is single-user (message_processor always
passes default_user), and the LangGraph half targets a backend slated for
retirement.

Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
@shoom1

shoom1 commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #110.

The framework review series includes this work in feat(workflow)!: SessionRef and user-scoped session APIs (68f8724), which generalizes it: every session hook takes an optional user_id (not just the ADK query/manage APIs), identity becomes the SessionRef(app_name, user_id, session_id) triple, the active turn is a ContextVar, and a backend with no durable store raises NotImplementedError rather than answering False/[].

Closing unmerged — the content is a strict subset of #110, not a dropped change.

@shoom1 shoom1 closed this Aug 3, 2026
@shoom1
shoom1 deleted the fix/adk-session-user-scope branch August 3, 2026 05:24
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.

1 participant