fix(desktop): bind new-chat workspace to active profile - #51117
fix(desktop): bind new-chat workspace to active profile#51117ThyFriendlyFox wants to merge 2 commits into
Conversation
Local desktop mode used one global workspace-cwd localStorage key, so switching Hermes profiles still seeded Cmd+N and session.create from another profile's folder. Scope remembered cwd per profile (matching remote), set workspace context on boot and gateway profile swap, and seed draft cwd from config.get project (terminal.cwd) when unset.
Co-authored-by: ThyFriendlyFox <thyfriendlyfox@gmail.com>
|
Thanks for the detailed writeup — but closing this, because the premise no longer holds against current The bug is moot on current export const workspaceCwdForNewSession = (): string => {
if ($connection.get()?.mode === 'remote') return getRememberedWorkspaceCwd()
// bare new chat starts DETACHED — no inherited cwd
return getConfiguredDefaultProjectDir()
}That "a bare new chat starts DETACHED" behavior is intentional (the first-class Projects transition): a plain new chat should not silently inherit the last folder you touched — entering a Project/worktree attaches its cwd explicitly instead. So the cross-profile leak you're fixing (the shared local key feeding What the PR would actually do is re-introduce per-profile remembered-folder restoration for new chats, which is the behavior that design deliberately removed. That's a product/design change ("new chats should resume each profile's last folder"), not a bug fix — and it'd need to be argued as such, since it reverses the detached-default intent. Remote new-sessions already key per-profile on (Also heads-up for future PRs: the branch carries a If you feel strongly that new chats should restore each profile's last folder, open an issue framing it as a UX proposal against the detached-default behavior and we can weigh it there. Appreciate the investigation either way. |
|
Thank you Brooklyn! The dummy commit was because Test 3 was unreliable and had a timeout, even though we didn't actually affect anything there. Will fix for the future |
Summary
New chats in the desktop client opened in the wrong folder after switching profiles. Selecting a profile correctly swapped its skills / memory / model, but a new chat still used the working directory from whatever profile you were last in. This binds the new-chat workspace to the active profile, with a one-time migration so existing users lose nothing.
Scope:
apps/desktoponly — 5 files, +132 / −16, one commit. No backend changes.The problem (vs
main)On
main, local desktop mode stored the remembered workspace folder under one globallocalStoragekey, shared by every profile. Remote mode already scoped its key per profile+backend — local mode just never did:Why that surfaces as a bug
The new-chat creation path (
use-session-actions.ts) resolves the cwd and ships it explicitly withsession.create:Two facts combine into the bug:
workspaceCwdForNewSession()read the single shared key, so it returned whatever folder you last used in any profile.cwdonsession.createwins over the gateway's ownterminal.cwdresolution for the bound profile.Net effect: in profile A you work in
~/reagent; you switch to profile B (whoseterminal.cwdis~/projectB) and start a new chat → it opens in~/reagent. The profile identity switched; the working directory was overridden by stale global state.The fix
Three coordinated, profile-agnostic changes. No profile name is hardcoded anywhere.
1. Scope the local key by profile —
store/session.tsworkspaceCwdKey()now appends the active workspace profile to the local key, mirroring what remote already did:A new
$workspaceProfileKeyatom +setWorkspaceProfileContext()setter tracks which profile's row we read/write. It's updated in three places so every entry point stays scoped:use-gateway-boot.ts) — seeds context from the profile the app launched under;store/profile.ts—selectProfile,newSessionInProfile,ensureGatewayProfile);2. Seed the draft on profile switch —
store/session.ts+desktop-controller.tsxNew
applyWorkspaceForActiveProfile(requestGateway)runs when the active gateway profile changes (wired into the existing profile-change effect indesktop-controller.tsx, alongsiderefreshCurrentModel). It:config.get { key: 'project', cwd }to also resolve the git branch), elseconfig.get { key: 'project' }, which falls back to that profile'sterminal.cwd;!$activeSessionId.get();session.createcan omit cwd and let the gateway resolve from the bound profile).3. One-time migration —
store/session.tsreadRememberedWorkspaceCwd()copies the legacy global key into thedefaultprofile's slot on first read, so upgrading users keep their existing workspace.Note: a bug I introduced and then removed mid-review
My first pass also reordered
workspaceCwdForNewSession()'s priority (remembered-before-configured). That silently broke the existing "configured default wins" contract — and the priority unit test failed on it. I reverted the reorder; it was never needed, because the profile-switch seeding sets$currentCwddirectly andsession.createreads$currentCwdfirst. Final ordering is unchanged frommain.Bug Fix Tests
All assertions below live in
apps/desktop/src/store/session.test.tsand ship with this PR.How to run
The tests (raw, as shipped)
Full verification matrix
vitest --environment jsdom— fullstore/+app/session/suitesctx-one/ctx-two)tsc -b --noEmitpadding-line-between-statementswarnings confirmed pre-existing onmaintui_gateway/server.pyconfig.get key=projectaccepts optionalcwd, falls back toterminal.cwd, returns{cwd, branch}requestGatewayconfirmed a stableuseCallback; new effect gated on!$activeSessionIdTemporary verification test (added during review, then removed)
The shipped tests cover the pure selector (
workspaceCwdForNewSession) and storage isolation, but the async seeding functionapplyWorkspaceForActiveProfilehad no direct coverage — its correctness rested on reasoning, not a test. To close that gap before opening this PR, I temporarily added adescribe('applyWorkspaceForActiveProfile')block tosession.test.tsthat mockedrequestGatewaywithvi.fn()and asserted all four code paths:config.get { key: 'project' }, seeds$currentCwd+$currentBranchfrom the profile'sterminal.cwd.config.get { key: 'project', cwd: <remembered> }and applies the validated cwd + branch.requestGatewayis not called and$currentCwdis left untouched.All four passed (suite went 21 → 25 green), empirically verifying the seeding path. I then removed the block and its now-unused imports so it stays out of the shipped diff —
git diffof the test file is byte-identical to the committed version. This documents that the path was tested even though the test isn't in the tree.The exact block that was run and then removed:
To reproduce: paste the block back into
apps/desktop/src/store/session.test.ts, add$currentBranchandapplyWorkspaceForActiveProfileto the imports from./session, then run the same command —cd apps/desktop npx vitest run --environment jsdom src/store/session.test.tsHonest caveat
Confidence here is structural and unit-level. This has not yet been exercised in a running desktop build with two real profiles clicked between live (
npm run dev). That final end-to-end confirmation is the one thing left for manual QA before merging upstream.Submitted with love from Team Reagent!