Skip to content

fix(api): dedup EndUser in plugin get_user by session_id for Reverse Invocation#36742

Merged
asukaminato0721 merged 5 commits into
langgenius:mainfrom
ShuntaroOkuma:fix-get-user-dedup-by-session-id
Jun 1, 2026
Merged

fix(api): dedup EndUser in plugin get_user by session_id for Reverse Invocation#36742
asukaminato0721 merged 5 commits into
langgenius:mainfrom
ShuntaroOkuma:fix-get-user-dedup-by-session-id

Conversation

@ShuntaroOkuma
Copy link
Copy Markdown
Contributor

Summary

get_user (plugin inner-api) creates a duplicate EndUser on every Reverse Invocation call: the non-anonymous branch looks up by EndUser.id == user_id, but the create path writes user_id into session_id (id is auto-generated). The id-only lookup never matches → fresh EndUser per call → conversations created on turn 1 cannot be looked up on turn 2 (ConversationService.get_conversationConversationNotExistsError → empty-message invalid_param 400). Multi-turn chat continuation from any Tool plugin via session.app.chat.invoke is broken.

This PR keeps the original "explicit EndUser.id wins" semantics for callers that pass a known id, and adds a session_id fallback so daemon-supplied session UUIDs dedup against the row created on the first call. Sequential rather than OR to avoid accidentally matching a different user whose session_id happens to equal the requested id (session_id is not unique).

Verification

Local docker-compose Dify 1.14.2 + a 2-turn Tool-plugin scenario:

  • Before: turn 1 succeeds, turn 2 fails with empty-message 400; DB shows two EndUsers with identical session_id and different id per run.
  • After: turn 2 succeeds and continues the same conversation; DB shows a single EndUser and a single Conversation row.

Known follow-up

end_user_tenant_session_id_idx is non-unique, so the select-then-insert path remains racy under concurrent calls — a pre-existing issue this PR does not address. Happy to follow up with a partial unique index + ON CONFLICT DO NOTHING if you'd like.

Fixes #36736

…Invocation

`get_user` (plugin inner-api) creates a duplicate EndUser on every
Reverse Invocation call: the non-anonymous branch looks up by
`EndUser.id == user_id`, but the create path writes `user_id` to
`session_id` (id is auto-generated). The lookup never matches → fresh
EndUser per call → the conversation owned by turn 1's EndUser cannot
be found by turn 2's EndUser → ConversationService.get_conversation
raises ConversationNotExistsError → handle_value_error returns an
empty-message invalid_param 400.

Broaden the non-anonymous lookup to match either column so daemon-
supplied session UUIDs dedup correctly.

Fixes langgenius#36736
@ShuntaroOkuma ShuntaroOkuma requested a review from WH-2099 as a code owner May 27, 2026 22:35
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label May 27, 2026
fatelei
fatelei previously approved these changes May 28, 2026
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 28, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 45.84% 45.78% -0.06%
Strict coverage 45.36% 45.31% -0.05%
Typed symbols 24,727 24,557 -170
Untyped symbols 29,525 29,385 -140
Modules 2764 2741 -23

@fatelei
Copy link
Copy Markdown
Contributor

fatelei commented May 28, 2026

test failed

- Update test_should_not_resolve_non_anonymous_users_across_tenants
  to expect 2 scalar calls (id lookup + session_id fallback) before
  the create path is reached.
- Add test_should_return_existing_user_by_session_id_fallback_for_non_anonymous
  pinning the new fallback behavior: id lookup misses, session_id
  lookup hits, no duplicate insert.
@ShuntaroOkuma
Copy link
Copy Markdown
Contributor Author

Updated the unit tests to follow the new lookup contract:

Ran locally in the api docker image: 18/18 pass. Could you re-approve the workflow run so the latest commit's CI can execute?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 28, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-05-31 15:08:07.625604528 +0000
+++ /tmp/pyrefly_pr.txt	2026-05-31 15:07:53.914377299 +0000
@@ -67,7 +67,7 @@
 ERROR Class member `PluginToolProviderController.entity` overrides parent class `BuiltinToolProviderController` in an inconsistent manner [bad-override-mutable-attribute]
   --> core/tools/plugin_tool/provider.py:12:5
 ERROR Cannot set item in `dict[str, dict[str, Any]]` [unsupported-operation]
-    --> core/tools/tool_manager.py:1110:58
+    --> core/tools/tool_manager.py:1108:58
 ERROR `(method: str, url: str, max_retries: int = ..., **kwargs: Any) -> httpx._models.Response` is not assignable to attribute `perform_request` with type `(self: CloudScraper, method: Unknown, url: Unknown, *args: Unknown, **kwargs: Unknown) -> requests.models.Response` [bad-assignment]
   --> core/tools/utils/web_reader_tool.py:66:35
 ERROR `list[Never]` is not assignable to attribute `tools` with type `Never` [bad-assignment]
@@ -1982,8 +1982,6 @@
   --> tests/unit_tests/commands/test_clean_expired_messages.py:94:9
 ERROR Expected a callable, got `None` [not-callable]
    --> tests/unit_tests/commands/test_clean_expired_messages.py:176:9
-ERROR `(self: list[Unknown], object: Unknown, /) -> None` is not assignable to attribute `echo` with type `(message: Any | None = None, file: IO[Any] | None = None, nl: bool = True, err: bool = False, color: bool | None = None) -> None` [bad-assignment]
-   --> tests/unit_tests/commands/test_data_migration_wizard.py:142:33
 ERROR Generator function should return `Generator` [bad-return]
   --> tests/unit_tests/commands/test_legacy_model_type_migration.py:36:38
 ERROR Object of class `FromClause` has no attribute `insert` [missing-attribute]
@@ -2037,19 +2035,19 @@
 ERROR Object of class `int` has no attribute `lower` [missing-attribute]
    --> tests/unit_tests/controllers/console/app/test_annotation_security.py:256:35
 ERROR Object of class `ModuleType` has no attribute `console_ns` [missing-attribute]
-  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:82:5
+  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:72:5
 ERROR Object of class `ModuleType` has no attribute `api` [missing-attribute]
-  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:83:5
+  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:73:5
 ERROR Object of class `ModuleType` has no attribute `bp` [missing-attribute]
-  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:84:5
+  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:74:5
 ERROR Object of class `ModuleType` has no attribute `app` [missing-attribute]
-  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:90:5
+  --> tests/unit_tests/controllers/console/app/test_app_response_models.py:80:5
 ERROR Argument `ModuleSpec | None` is not assignable to parameter `spec` with type `ModuleSpec` in function `_frozen_importlib.module_from_spec` [bad-argument-type]
-   --> tests/unit_tests/controllers/console/app/test_app_response_models.py:119:36
+   --> tests/unit_tests/controllers/console/app/test_app_response_models.py:109:36
 ERROR Object of class `NoneType` has no attribute `loader` [missing-attribute]
-   --> tests/unit_tests/controllers/console/app/test_app_response_models.py:122:12
+   --> tests/unit_tests/controllers/console/app/test_app_response_models.py:112:12
 ERROR Object of class `object` has no attribute `exec_module` [missing-attribute]
-   --> tests/unit_tests/controllers/console/app/test_app_response_models.py:123:5
+   --> tests/unit_tests/controllers/console/app/test_app_response_models.py:113:5
 ERROR Missing argument `app_model` in function `handler` [missing-argument]
    --> tests/unit_tests/controllers/console/app/test_wraps.py:104:19
 ERROR Unexpected keyword argument `app_id` in function `handler` [unexpected-keyword]
@@ -2109,9 +2107,9 @@
 ERROR Argument `Flask` is not assignable to parameter `app` with type `DifyApp` in function `extensions.ext_fastopenapi.init_app` [bad-argument-type]
   --> tests/unit_tests/controllers/console/test_fastopenapi_version.py:23:30
 ERROR Argument `SimpleNamespace` is not assignable to parameter `form` with type `Form` in function `controllers.console.human_input_form._jsonify_form_definition` [bad-argument-type]
-  --> tests/unit_tests/controllers/console/test_human_input_form.py:37:41
+  --> tests/unit_tests/controllers/console/test_human_input_form.py:35:41
 ERROR Argument `SimpleNamespace` is not assignable to parameter `form` with type `Form` in function `controllers.console.human_input_form.ConsoleHumanInputFormApi._ensure_console_access` [bad-argument-type]
-  --> tests/unit_tests/controllers/console/test_human_input_form.py:49:57
+  --> tests/unit_tests/controllers/console/test_human_input_form.py:47:57
 ERROR Object of class `Flask` has no attribute `login_manager` [missing-attribute]
   --> tests/unit_tests/controllers/console/test_workspace_account.py:29:5
 ERROR `SimpleNamespace | object` is not assignable to attribute `_current_tenant` with type `Tenant | None` [bad-assignment]
@@ -2120,8 +2118,6 @@
   --> tests/unit_tests/controllers/console/test_workspace_members.py:16:5
 ERROR `SimpleNamespace` is not assignable to attribute `_current_tenant` with type `Tenant | None` [bad-assignment]
   --> tests/unit_tests/controllers/console/test_workspace_members.py:73:43
-ERROR `in` is not supported between `Literal['count']` and `None` [not-iterable]
-   --> tests/unit_tests/controllers/console/test_wraps.py:213:16
 ERROR `SimpleNamespace` is not assignable to attribute `db` with type `SQLAlchemy` [bad-assignment]
   --> tests/unit_tests/controllers/files/test_image_preview.py:23:17
 ERROR `SimpleNamespace` is not assignable to attribute `request` with type `Request` [bad-assignment]
@@ -2159,37 +2155,37 @@
 ERROR Could not find name `Import` [unknown-name]
    --> tests/unit_tests/controllers/inner_api/app/test_dsl.py:120:71
 ERROR Missing argument `tenant_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:212:44
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:242:44
 ERROR Missing argument `user_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:212:44
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:242:44
 ERROR Missing argument `tenant_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:229:31
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:259:31
 ERROR Missing argument `user_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:229:31
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:259:31
 ERROR Missing argument `tenant_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:244:35
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:274:35
 ERROR Missing argument `user_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:244:35
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:274:35
 ERROR Missing argument `tenant_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:268:44
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:298:44
 ERROR Missing argument `user_model` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:268:44
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:298:44
 ERROR Argument `type[PluginTestPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:296:35
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:326:35
 ERROR Missing argument `payload` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:302:36
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:332:36
 ERROR Argument `type[PluginTestPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:311:35
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:341:35
 ERROR Missing argument `payload` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:318:31
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:348:31
 ERROR Argument `type[TestPluginData.test_should_raise_error_on_invalid_payload.InvalidPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:329:35
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:359:35
 ERROR Missing argument `payload` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:336:31
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:366:31
 ERROR Argument `type[PluginTestPayload]` is not assignable to parameter `payload_type` with type `type[BaseModel]` in function `controllers.inner_api.plugin.wraps.plugin_data` [bad-argument-type]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:342:35
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:372:35
 ERROR Missing argument `payload` in function `protected_view` [missing-argument]
-   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:348:36
+   --> tests/unit_tests/controllers/inner_api/plugin/test_plugin_wraps.py:378:36
 ERROR Object of class `FunctionType` has no attribute `view_class` [missing-attribute]
   --> tests/unit_tests/controllers/openapi/test_account.py:40:12
 ERROR Object of class `FunctionType` has no attribute `view_class` [missing-attribute]
@@ -2348,6 +2344,28 @@
    --> tests/unit_tests/controllers/service_api/dataset/rag_pipeline/test_rag_pipeline_workflow.py:328:33
 ERROR Missing argument `response_mode` in function `services.rag_pipeline.entity.pipeline_service_api_entities.PipelineRunApiEntity.__init__` [missing-argument]
    --> tests/unit_tests/controllers/service_api/dataset/rag_pipeline/test_rag_pipeline_workflow.py:328:33
+ERROR Argument `list[dict[str, Any]] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+  --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:52:20
+ERROR `None` is not subscriptable [unsupported-operation]
+  --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:75:16
+ERROR `None` is not subscriptable [unsupported-operation]
+  --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:76:16
+ERROR Missing argument `content` in function `controllers.common.controller_schemas.ChildChunkCreatePayload.__init__` [missing-argument]
+   --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:120:36
+ERROR Argument value `Literal[0]` violates Pydantic `ge` constraint `Literal[1]` for field `limit` [bad-argument-type]
+   --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:160:33
+ERROR Argument value `Literal[0]` violates Pydantic `ge` constraint `Literal[1]` for field `page` [bad-argument-type]
+   --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:165:33
+ERROR Argument `list[DocumentSegment] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+   --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:295:20
+ERROR Missing argument `tenant_id` in function `services.dataset_service.SegmentService.get_segments` [missing-argument]
+   --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:304:54
+ERROR Argument `list[str] | None` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+   --> tests/unit_tests/controllers/service_api/dataset/test_dataset_segment.py:597:20
+ERROR Object of class `NoneType` has no attribute `name` [missing-attribute]
+   --> tests/unit_tests/controllers/service_api/dataset/test_document.py:238:16
+ERROR Object of class `NoneType` has no attribute `indexing_status` [missing-attribute]
+   --> tests/unit_tests/controllers/service_api/dataset/test_document.py:239:16
 ERROR Missing argument `app_model` in function `protected_view` [missing-argument]
    --> tests/unit_tests/controllers/service_api/test_wraps.py:160:36
 ERROR `object` is not assignable to attribute `request` with type `Request` [bad-assignment]
@@ -6173,18 +6191,6 @@
   --> tests/unit_tests/services/auth/test_auth_type.py:81:34
 ERROR Argument `dict[str, str]` is not assignable to parameter `credentials` with type `AuthCredentials` in function `services.auth.jina.jina.JinaAuth.__init__` [bad-argument-type]
   --> tests/unit_tests/services/auth/test_jina_auth.py:35:22
-ERROR Class member `CapturingImportService._import_workflows` overrides parent class `MigrationImportService` in an inconsistent manner [bad-override]
-   --> tests/unit_tests/services/data_migration/test_import_service.py:129:13
-ERROR Argument `object` is not assignable to parameter `account` with type `Account` in function `services.data_migration.import_service.MigrationImportService._import_workflow_app` [bad-argument-type]
-   --> tests/unit_tests/services/data_migration/test_import_service.py:231:17
-ERROR Class member `PublishingImportService._find_existing_app` overrides parent class `MigrationImportService` in an inconsistent manner [bad-override]
-   --> tests/unit_tests/services/data_migration/test_import_service.py:333:13
-ERROR Class member `StrategyImportService._find_existing_app` overrides parent class `MigrationImportService` in an inconsistent manner [bad-override]
-   --> tests/unit_tests/services/data_migration/test_import_service.py:400:13
-ERROR Class member `SkipImportService._find_existing_app` overrides parent class `MigrationImportService` in an inconsistent manner [bad-override]
-   --> tests/unit_tests/services/data_migration/test_import_service.py:465:13
-ERROR Class member `OrderedImportService._import_workflows` overrides parent class `MigrationImportService` in an inconsistent manner [bad-override]
-   --> tests/unit_tests/services/data_migration/test_import_service.py:922:13
 ERROR Argument `str` is not assignable to parameter `indexing_technique` with type `Literal['economy', 'high_quality']` in function `services.entities.knowledge_entities.rag_pipeline_entities.KnowledgeConfiguration.__init__` [bad-argument-type]
    --> tests/unit_tests/services/dataset_service_test_helpers.py:447:28
 ERROR Unexpected keyword argument `workspace_id` in function `services.enterprise.enterprise_service.DefaultWorkspaceJoinResult.__init__` [unexpected-keyword]
@@ -6575,11 +6581,11 @@
 ERROR Argument `dict[str, dict[str, str | dict[str, str]] | str]` is not assignable to parameter `object` with type `dict[str, dict[str, list[Unknown] | str] | str]` in function `list.append` [bad-argument-type]
    --> tests/unit_tests/services/test_workflow_service.py:224:13
 ERROR Missing required key `id` for TypedDict `NodeConfigDict` [bad-typed-dict-key]
-    --> tests/unit_tests/services/test_workflow_service.py:2783:71
+    --> tests/unit_tests/services/test_workflow_service.py:2755:71
 ERROR Missing required key `data` for TypedDict `NodeConfigDict` [bad-typed-dict-key]
-    --> tests/unit_tests/services/test_workflow_service.py:2783:71
+    --> tests/unit_tests/services/test_workflow_service.py:2755:71
 ERROR Argument `dict[str, str | dict[str, str]]` is not assignable to parameter `node_config` with type `NodeConfigDict` in function `services.workflow_service.WorkflowService._build_human_input_node` [bad-argument-type]
-    --> tests/unit_tests/services/test_workflow_service.py:2870:65
+    --> tests/unit_tests/services/test_workflow_service.py:2842:65
 ERROR Argument `Literal['api_key']` is not assignable to parameter `credential_type` with type `CredentialType` in function `services.tools.builtin_tools_manage_service.BuiltinToolManageService.list_builtin_provider_credentials_schema` [bad-argument-type]
   --> tests/unit_tests/services/tools/test_builtin_tools_manage_service.py:91:89
 ERROR Object of class `Mapping` has no attribute `startswith` [missing-attribute]

@asukaminato0721 asukaminato0721 enabled auto-merge May 31, 2026 15:06
@ShuntaroOkuma ShuntaroOkuma requested a review from fatelei June 1, 2026 00:35
@asukaminato0721 asukaminato0721 added this pull request to the merge queue Jun 1, 2026
Merged via the queue into langgenius:main with commit e7be04f Jun 1, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reverse Invocation chat continuation fails: get_user creates duplicate EndUser per call (lookup-vs-create column mismatch)

3 participants