[chore] Remove the /load-session endpoint and dead session-store scaffolding#4828
Merged
Conversation
dosubot
Bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
refactoring
📝 WalkthroughWalkthroughThis PR removes the ChangesLoad-session surface removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…e scaffolding /load-session was a shell over NoopSessionStore that always returned HTTP 200 with an empty message list. That lies to clients: an empty conversation reads as "no history yet", not "history is not implemented". Durable session storage was never built, so the route, its request/response models, and the SessionStore port behind it were all dead scaffolding. Remove: - the /load-session route and make_load_session_endpoint helper, and drop the session_store parameter from register_agent_message_routes (keep /messages) - LoadSessionRequest / LoadSessionResponse models - the SessionStore and NoopSessionStore ports (used only by /load-session; the /messages session-id path validates and echoes the id and never touches a store, and nothing calls save_turn) and their top-level re-exports - "load-session" from the reserved agent paths The runtime stays cold: the client sends full history on every turn. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
Keep the docs in sync with the code removal in the same change. Delete the public-edge/agent-load-session.md page and its README bullet, remove the /load-session row and section from documentation/protocol.md, the SessionStore section/entries from ports-and-adapters.md, and the load-session footnote + SessionStore status from the interfaces inventory (README index row, runtime-ports, browser-protocol-adapter). Update ground-truth, architecture, sessions, the agent-workflows README, and the opencode research note to describe the runtime as cold with no durable store and no history-load endpoint. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
Add the lane row, record the SessionStore-removal decision and the stacking on the interface-inventory lane, and release the BUT-LOCK. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
mmabrouk
force-pushed
the
docs/agent-workflow-interface-inventory
branch
from
5dd60f4 to
90930b9
Compare
mmabrouk
force-pushed
the
chore/agent-remove-load-session
branch
from
2ee7422 to
8d40cff
Compare
Contributor
Railway Preview Environment
Updated at 2026-06-25T10:54:51.765Z |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdks/python/oss/tests/pytest/utils/test_routing.py (1)
140-172: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that
/load-sessionis gone from the agent OpenAPI.These tests only prove
/messagesis present. If/load-sessionwere still registered, they would still pass, so the removal contract is no longer enforced.Suggested test hardening
schema = _mounts(app)["/chat"].app.openapi() assert "/messages" in schema["paths"] + assert "/load-session" not in schema["paths"] assert "/invoke" in schema["paths"] # the base routes are still present @@ schema = app.openapi() assert "/messages" in schema["paths"] + assert "/load-session" not in schema["paths"]
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cac82487-7569-4355-b5ba-6958849bd8ca
📒 Files selected for processing (21)
docs/design/agent-workflows/README.mddocs/design/agent-workflows/documentation/architecture.mddocs/design/agent-workflows/documentation/ground-truth.mddocs/design/agent-workflows/documentation/ports-and-adapters.mddocs/design/agent-workflows/documentation/protocol.mddocs/design/agent-workflows/documentation/sessions.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/in-service/browser-protocol-adapter.mddocs/design/agent-workflows/interfaces/in-service/runtime-ports.mddocs/design/agent-workflows/interfaces/public-edge/README.mddocs/design/agent-workflows/interfaces/public-edge/agent-load-session.mddocs/design/agent-workflows/projects/research/opencode-architecture.mddocs/design/agent-workflows/scratch/agent-coordination.mdsdks/python/agenta/sdk/agents/__init__.pysdks/python/agenta/sdk/agents/adapters/vercel/routing.pysdks/python/agenta/sdk/agents/interfaces.pysdks/python/agenta/sdk/decorators/routing.pysdks/python/agenta/sdk/models/workflows.pysdks/python/oss/tests/pytest/utils/test_messages_endpoint.pysdks/python/oss/tests/pytest/utils/test_routing.pyservices/oss/src/agent/app.py
💤 Files with no reviewable changes (5)
- docs/design/agent-workflows/interfaces/public-edge/README.md
- docs/design/agent-workflows/interfaces/public-edge/agent-load-session.md
- sdks/python/agenta/sdk/agents/interfaces.py
- sdks/python/agenta/sdk/models/workflows.py
- sdks/python/agenta/sdk/agents/init.py
| ## Verification Pointers | ||
|
|
||
| - `/messages` and `/load-session` routing tests live in | ||
| - `/messages` routing tests live in |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include the route-registration test here too.
sdks/python/oss/tests/pytest/utils/test_messages_endpoint.py covers the endpoint flow, but sdks/python/oss/tests/pytest/utils/test_routing.py also asserts /messages in the OpenAPI schema. Listing both keeps the verification path complete. As per the provided test snippets, test_routing.py still guards /messages registration.
Suggested edit
-- `/messages` routing tests live in
- `sdks/python/oss/tests/pytest/utils/test_messages_endpoint.py`.
+- `/messages` routing tests live in
+ `sdks/python/oss/tests/pytest/utils/test_messages_endpoint.py`
+ and `sdks/python/oss/tests/pytest/utils/test_routing.py`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `/messages` routing tests live in | |
| - `/messages` routing tests live in | |
| `sdks/python/oss/tests/pytest/utils/test_messages_endpoint.py` | |
| and `sdks/python/oss/tests/pytest/utils/test_routing.py`. |
Comment on lines
234
to
241
| class TestReservedAgentPaths: | ||
| def test_agent_endpoint_names_are_reserved(self): | ||
| assert {"messages", "load-session"} <= _RESERVED_PATHS | ||
| assert {"messages"} <= _RESERVED_PATHS | ||
|
|
||
| @pytest.mark.parametrize("reserved", ["messages", "load-session"]) | ||
| @pytest.mark.parametrize("reserved", ["messages"]) | ||
| def test_route_rejects_reserved_agent_path(self, reserved): | ||
| with pytest.raises(ValueError, match=reserved): | ||
| route(f"/{reserved}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Check that load-session is no longer reserved.
Line 236 now checks only a subset, so this still passes if "load-session" accidentally remains in _RESERVED_PATHS.
Suggested test hardening
class TestReservedAgentPaths:
def test_agent_endpoint_names_are_reserved(self):
assert {"messages"} <= _RESERVED_PATHS
+ assert "load-session" not in _RESERVED_PATHS📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| class TestReservedAgentPaths: | |
| def test_agent_endpoint_names_are_reserved(self): | |
| assert {"messages", "load-session"} <= _RESERVED_PATHS | |
| assert {"messages"} <= _RESERVED_PATHS | |
| @pytest.mark.parametrize("reserved", ["messages", "load-session"]) | |
| @pytest.mark.parametrize("reserved", ["messages"]) | |
| def test_route_rejects_reserved_agent_path(self, reserved): | |
| with pytest.raises(ValueError, match=reserved): | |
| route(f"/{reserved}") | |
| class TestReservedAgentPaths: | |
| def test_agent_endpoint_names_are_reserved(self): | |
| assert {"messages"} <= _RESERVED_PATHS | |
| assert "load-session" not in _RESERVED_PATHS | |
| `@pytest.mark.parametrize`("reserved", ["messages"]) | |
| def test_route_rejects_reserved_agent_path(self, reserved): | |
| with pytest.raises(ValueError, match=reserved): | |
| route(f"/{reserved}") |
mmabrouk
changed the base branch from
docs/agent-workflow-interface-inventory
to
big-agents
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
1 participant
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.
Context
POST /load-sessionwas a shell. It ran overNoopSessionStoreand always returned HTTP 200 with an empty message list, so a client could not tell "this session has no history yet" from "session history is not implemented". Durable session storage was never built, which left the route, its request/response models, and theSessionStoreport behind it as dead scaffolding that read as a real, working contract.We decided to remove it rather than keep faking the contract.
Changes
Removed the endpoint and everything that existed only to serve it.
adapters/vercel/routing.py: deleted the/load-sessionroute andmake_load_session_endpoint, and dropped thesession_storeparameter fromregister_agent_message_routes./messagesis untouched.models/workflows.py: deletedLoadSessionRequestandLoadSessionResponse.agents/interfaces.py: deleted theSessionStoreandNoopSessionStoreports, plus their top-level re-exports inagents/__init__.py.decorators/routing.py: dropped"load-session"from_RESERVED_PATHS.The runtime stays cold: the client sends the full conversation on every turn, and
/messagesstill validates, mints, and echoes thesession_id(it never touched a store).SessionStore decision
Removed the port entirely. A grep across
sdks/python/agenta/andservices/oss/src/showedSessionStore/NoopSessionStore/save_turnhad exactly one caller: the/load-sessionroute. The/messagessession-id path works on the id string alone and never reads or writes a store, and nothing callssave_turn. With the route gone, the port is dead, so it goes too. When durable history is built, it will get a fresh port plus a write path and a real load endpoint.Docs in the same change
interfaces/public-edge/agent-load-session.mdand its bullet inpublic-edge/README.md./load-sessionrow and section fromdocumentation/protocol.mdand theSessionStorematerial fromdocumentation/ports-and-adapters.md.SessionStorestatus from the interface inventory (interfaces/README.mdindex row,runtime-ports.md,browser-protocol-adapter.md).ground-truth.md,architecture.md,sessions.md, the agent-workflowsREADME.md, and the OpenCode research note to describe the runtime as cold with no durable store and no history-load endpoint.Scope / risk
/messagesis not touched. The cold-session contract (caller sends full conversation on each turn) is unchanged. The only thing removed is dead code and the scaffolding built for a never-implemented feature. Any code that was importingSessionStoreorNoopSessionStorefrom the SDK will break at import time after this PR; a grep confirmed no such caller exists outside the deleted route.Tests / notes
/load-sessiontests intest_messages_endpoint.pyand the/load-sessionassertions and reserved-path cases intest_routing.py.ruff formatandruff checkclean on every touched file.docs/agent-workflow-interface-inventory([docs] Add agent workflow interface inventory #4821): that lane first-committed the interface inventory pages andprotocol.md/ports-and-adapters.md, so this PR is based on it and should merge after it.How to QA
Prerequisites: Local dev stack (
docker compose uporrun.sh). The agent service must be running (services/oss/+ the SDK installed).Steps:
POST /load-sessionreturns 404, not 200:POST /messagesstill works (not affected):SessionStoreorNoopSessionStore:Expected result: Step 1 returns 404. Step 2 returns 200 with a valid session echo. Step 3 raises
ImportError.Automated tests:
Edge cases: Confirm no existing agent app that imports from
agenta.sdk.agentsbreaks. The import surface changed; verifyfrom agenta import Agentstill works cleanly.https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc