fix(desktop): pass owning profile to server on session delete/archive#44138
Open
AIalliAI wants to merge 3 commits into
Open
fix(desktop): pass owning profile to server on session delete/archive#44138AIalliAI wants to merge 3 commits into
AIalliAI wants to merge 3 commits into
Conversation
The desktop sidebar lists every profile's sessions straight from each profile's on-disk state.db (GET /api/profiles/sessions), but deleteSession() only passed the owning profile to Electron's backend router — never to the server. When the serving process is scoped to a different profile than the desktop believes (the sticky active_profile file, flipped by "Set as active" on the Profiles page, is honored on a legacy launch with no --profile flag), the DELETE looks up the session in the wrong profile's state.db and 404s with "Session not found". Reads already work because getSessionMessages() passes ?profile=, and rename works because it puts profile in the PATCH body — delete and archive were the odd ones out. Align them: - deleteSession(): append ?profile= to the path (the endpoint already accepts it), keeping request.profile for Electron's process routing. - setSessionArchived(): include profile in the PATCH body, matching renameSession() (the endpoint already reads body.profile). - delete_session_endpoint: resolve the session id via resolve_session_id() first, for parity with its GET/PATCH siblings. Fixes NousResearch#44117 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
12 tasks
1 task
Regression test for the archive half of NousResearch#44117, prompted by a report that the PATCH allowlist might 400 on 'archived'. That allowlist is in gateway/platforms/api_server.py (the OpenAI-compatible platform API), which the desktop never calls; the dashboard server's PATCH handler already supports archived + profile, and this test pins that down. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ete-profile # Conflicts: # apps/desktop/src/hermes.test.ts
Contributor
Author
|
Requesting maintainer review — this is ready to land from my side. Standalone fork CI is pending first-run approval here; the rollup branch in #44061 carrying this session's batch is fully green on upstream CI (all test shards, typecheck, e2e). |
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
2 participants
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.
Fixes #44117
Problem
Deleting a session in the default profile from the desktop sidebar fails with "Delete failed — Session not found", and the session stays in the list.
Root cause
The desktop sidebar lists sessions via
GET /api/profiles/sessions, which the primary backend serves by reading every profile'sstate.dbdirectly from disk — so it always shows each profile's rows regardless of which profile the serving process itself runs as.The mutations, however, are inconsistent about telling the server which profile owns the session:
getSessionMessages)?profile=query paramrenameSession)profilein PATCH bodydeleteSession)request.profile(Electron's backend-process router)setSessionArchived)request.profileWhen the dashboard process is actually scoped to a different profile than the desktop believes — e.g. after "Set as active" on the Profiles page, which flips the sticky
active_profilefile that the next legacy desktop launch (no--profileflag, see_apply_profile_overridestep 2) honors —DELETE /api/sessions/{id}opens the serving process's ownstate.db, doesn't find the row, and 404s. The same profile-confusion class was already fixed for the skills/toolsets endpoints (see the_profile_scopecomment block inhermes_cli/web_server.py); session delete/archive were missed.Fix
Match the established read/rename pattern — pass the owning profile to the server, not just to Electron's process router:
deleteSession(): append?profile=to the path (the endpoint already accepts it).request.profileis kept so per-profile remote overrides still route correctly;interceptSessionRequestForRemotereads the param fromsearchParamsand global-remote mode re-appends it, so both remote shapes are unaffected.setSessionArchived(): includeprofilein the PATCH body, matchingrenameSession()(the endpoint'sSessionRenamemodel already readsbody.profile).delete_session_endpoint: resolve the id viaresolve_session_id()first, for parity with its GET/PATCH siblings (they all resolve; DELETE was the only one doing a raw exact match).Tests
tests/hermes_cli/test_web_server.py: DELETE with?profile=deletes from that profile'sstate.db(regression test for this issue); DELETE resolves unique id prefixes; unknown id still 404s. Full file passes (265 passed).apps/desktop/src/hermes.test.ts: delete sends?profile=+request.profile; no param without an owning profile; archive sendsbody.profile. (vitest, jsdom)tsc -bclean.🤖 Generated with Claude Code