Skip to content

fix: eagerly validate conversation on service-api and explore chat endpoints#38801

Merged
fatelei merged 2 commits into
langgenius:mainfrom
he-yufeng:fix/eager-conversation-validation-remaining-endpoints
Jul 12, 2026
Merged

fix: eagerly validate conversation on service-api and explore chat endpoints#38801
fatelei merged 2 commits into
langgenius:mainfrom
he-yufeng:fix/eager-conversation-validation-remaining-endpoints

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #37224, which fixed the "hang on non-existing conversation_id" bug (#37197) on the web chat-messages endpoint. The merged commit (31a50a3b20) only changed controllers/web/completion.py, although its description also listed the Service API and console endpoints. As a result the same hang still reproduces on two other chat entry points:

  • controllers/service_api/app/completion.pyChatApi
  • controllers/console/explore/completion.pyChatApi

Both pass a user-supplied conversation_id straight into the streaming generator, where ConversationNotExistsError is raised lazily — after the controller's try/except has already returned the SSE response — so an invalid id hangs on ping events instead of returning 404.

This adds the same eager ConversationService.get_conversation() pre-check before AppGenerateService.generate() on both endpoints. The lookup is identical to the one the generate pipeline already runs at entry (core/app/apps/chat/app_generator.py — same app_model / user / session=db.session() arguments), so valid conversations are unaffected; it only moves the existing check earlier so the error surfaces synchronously and is mapped to 404.

controllers/console/app/completion.py (the debugger ChatMessageApi) was also named in #37224 but routes through _create_chat_message, which already validates the id for AGENT apps and uses a different error-mapping path; it's left as a separate follow-up (tracked in #38800) to keep this change focused and low-risk.

Fixes #38800

Screenshots

N/A — backend error-handling change (an invalid conversation_id now returns 404 instead of hanging on SSE ping events).

Test plan

  • Added test_invalid_conversation_id_fails_fast_as_not_found to both endpoints' unit tests: an invalid conversation_id returns 404, and AppGenerateService.generate is never reached (assert_not_called).
  • Existing behavior is untouched: the check is guarded by if payload.conversation_id:, so the no-id and valid-id paths are unchanged.
  • Ran locally on the touched files: ruff check clean; both controller test suites pass (74 passed). I did not run the full make type-check locally; CI will cover it.

Checklist

  • This change requires a documentation update
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

From Claude Code

…dpoints

langgenius#37224 fixed the hang on the web chat-messages endpoint by validating the
conversation before AppGenerateService.generate(), but the merged change only
touched controllers/web/completion.py. The same non-existing conversation_id
still hangs on the Service API and Explore chat endpoints: both pass a
user-supplied conversation_id straight into the streaming generator, where
ConversationNotExistsError is raised lazily and can no longer be turned into a
clean 4xx by the surrounding try/except.

Add the same eager ConversationService.get_conversation() pre-check before
generate() on both endpoints so an invalid id fails fast as 404. The lookup
mirrors the one the generate pipeline already runs at entry
(core/app/apps/chat/app_generator.py), so valid conversations are unaffected.

issue: langgenius#37197

From Claude Code

Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 12, 2026
@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-07-12 12:46:04.422424881 +0000
+++ /tmp/pyrefly_pr.txt	2026-07-12 12:45:54.078256717 +0000
@@ -2253,7 +2253,7 @@
 ERROR `None` is not subscriptable [unsupported-operation]
    --> tests/unit_tests/controllers/service_api/app/test_audio.py:186:16
 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/app/test_completion.py:154:20
+   --> tests/unit_tests/controllers/service_api/app/test_completion.py:155:20
 ERROR Argument value `Literal[0]` violates Pydantic `ge` constraint `Literal[1]` for field `limit` [bad-argument-type]
   --> tests/unit_tests/controllers/service_api/app/test_conversation.py:82:35
 ERROR Argument value `Literal[101]` violates Pydantic `le` constraint `Literal[100]` for field `limit` [bad-argument-type]

@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 52.85% 52.85% +0.00%
Strict coverage 52.36% 52.36% +0.00%
Typed symbols 32,803 32,808 +5
Untyped symbols 29,539 29,541 +2
Modules 2988 2988 0

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jul 12, 2026
@fatelei fatelei added this pull request to the merge queue Jul 12, 2026
Merged via the queue into langgenius:main with commit fde08d2 Jul 12, 2026
31 checks passed
BlueSkyXN added a commit to BlueSkyXN/dify that referenced this pull request Jul 12, 2026
* fix(web): improve marketplace install dialog accessibility (langgenius#38798)

* refactor(web): normalize z-index layering (langgenius#38796)

* fix(web): correct knowledge dialog draft state (langgenius#38794)

* fix(agent-v2): restore keyboard access to row actions (langgenius#38792)

* fix(web): split model selection and settings actions (langgenius#38797)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* fix: eagerly validate conversation on service-api and explore chat endpoints (langgenius#38801)

Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

---------

Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Co-authored-by: yyh <92089059+lyzno1@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
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.

Chat-messages hang on non-existing conversation_id still reproduces on Service API and Explore endpoints (follow-up to #37224)

2 participants