fix(chat): route /api/repos/{repo_id}/chat/messages by workspace_sessions#146
Open
enyr-srl wants to merge 1 commit intorepowise-dev:mainfrom
Open
fix(chat): route /api/repos/{repo_id}/chat/messages by workspace_sessions#146enyr-srl wants to merge 1 commit intorepowise-dev:mainfrom
enyr-srl wants to merge 1 commit intorepowise-dev:mainfrom
Conversation
…ions
In workspace mode every non-primary repo's chat returns
``404 Repository <id> not found`` even though the same id is
listed in both ``GET /api/repos`` and
``GET /api/workspace.repos[].repo_id``. The chat router was the
only ``/api/repos/{repo_id}/...`` endpoint not honouring
``app.state.workspace_sessions``.
The conversation endpoints (``GET /chat/conversations``, etc.)
already work correctly because they use ``Depends(get_db_session)``,
which routes by ``repo_id`` automatically. ``chat_messages``
couldn't use that dep — it needs the *factory* (not a single
session) so it can open multiple sessions over the SSE lifetime —
so it was hand-wiring ``request.app.state.session_factory`` and
missing the workspace branch.
Fix:
* Extract the factory-resolution logic from ``get_db_session``
into two reusable helpers in ``deps.py``:
- ``resolve_session_factory(app_state, repo_id)`` — pure
function, same logic as the existing private helper in
``routers/repos.py``.
- ``resolve_request_session_factory(request)`` —
request-scoped sibling that reads ``repo_id`` from
path/query params (mirrors what ``get_db_session`` encodes).
* ``chat_messages`` calls
``resolve_request_session_factory(request)`` instead of
``request.app.state.session_factory`` directly.
* ``routers/repos.py::_resolve_repo_session_factory`` is now a
one-line alias around the shared helper, removing the duplicate
logic.
Test:
* New regression test ``tests/unit/server/test_chat_workspace.py``:
- ``test_chat_messages_resolves_non_primary_repo_in_workspace_mode``
— POST to a non-primary ``repo_id`` no longer returns 404.
Verified to fail on the unpatched source with the exact
``Repository <id> not found`` message.
- ``test_chat_messages_still_finds_primary_repo`` — the
single-factory fallback still works when
``workspace_sessions`` is empty (single-repo mode and the
primary repo of a workspace).
* All 126 existing ``tests/unit/server/`` tests pass with the patch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(chat): route
/api/repos/{repo_id}/chat/messagesby workspace_sessionsSummary
In workspace mode, every non-primary repo's chat returns
404 Repository <id> not foundeven though the same id is listedin both
GET /api/reposandGET /api/workspace.repos[].repo_id.The chat router was the only
/api/repos/{repo_id}/...endpointnot honouring
app.state.workspace_sessions.The conversation endpoints (
GET /chat/conversations, etc.)already work correctly because they use
Depends(get_db_session),which routes by
repo_idautomatically.chat_messagescouldn'tuse that dep — it needs the factory (not a single session) so it
can open multiple sessions over the SSE lifetime — so it was
hand-wiring
request.app.state.session_factoryand missing theworkspace branch.
Fix
get_db_sessionintotwo reusable helpers in
deps.py:resolve_session_factory(app_state, repo_id)— purefunction, same logic as the existing private helper in
routers/repos.py.resolve_request_session_factory(request)— request-scopedsibling that reads
repo_idfrom path/query params (mirrorswhat
get_db_sessionencodes).chat_messagescallsresolve_request_session_factory(request)instead of
request.app.state.session_factorydirectly.routers/repos.py::_resolve_repo_session_factoryis now aone-line alias around the shared helper, removing the duplicate
logic.
Tests
New regression test
tests/unit/server/test_chat_workspace.py:test_chat_messages_resolves_non_primary_repo_in_workspace_mode— POST to a non-primary
repo_idno longer returns 404.Verified to fail on the unpatched source with the exact
Repository <id> not foundmessage.test_chat_messages_still_finds_primary_repo— thesingle-factory fallback still works when
workspace_sessionsisempty (single-repo mode and the primary repo of a workspace).
All 126 existing
tests/unit/server/tests pass with the patch(verified locally).
Repro (against an unpatched workspace deployment)
The same
<non-primary-id>appears inGET /api/workspace.repos[].repo_id.Test plan
user-visible 404 message
tests/unit/server/tests still passinstall (reviewer)
Closes #145.