fix(desktop): honor default project directory for new sessions#43234
Conversation
The Settings picker persisted project-dir.json but the renderer kept seeding new chats from sticky localStorage home. Prefer the configured default on boot and session.create, pin TERMINAL_CWD at backend spawn, and reject packaged install-dir paths that regressed after #37536. Co-authored-by: Cursor <cursoragent@cursor.com>
🔎 Lint report:
|
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good:
- Fixes desktop app honoring the configured default project directory for new sessions. Previously the Electron preload or main process was not properly passing the configured project directory to session creation, causing new sessions to default to the wrong directory.
- 8 files, +158/-10. Touches Electron main/preload, React hooks, i18n, and store code.
- Test added for the config-driven project directory.
- Clean, well-scoped fix.
Reviewed by Hermes Agent
OutThisLife
left a comment
There was a problem hiding this comment.
Code Review
Verdict: Comment (not blocking the design — the root-cause fix is right — but two things should land before merge)
Looks good
- Correct root cause: the renderer was seeding
$currentCwdfrom stickylocalStorageand never preferring the configured default. Wiring it through boot (ensureDefaultWorkspaceCwd), the new-chat draft, andsession.create(workspaceCwdForNewSession()) is the right place to fix it. - Pinning
TERMINAL_CWD: hermesCwdon both backend spawns closes the stale-config-bridge gap so tools don't drift back to the install dir. Good catch. - Defense-in-depth in
main.cjsis sound: droppingINIT_CWD/process.cwd()when packaged and rejecting install-tree paths viaisPackagedInstallPath()directly addresses the #37536 item 16 regression. - Nice consistency cleanup making
defaultLabel/i18n report home (~) instead of the phantom~/hermes-projectsthatresolveHermesCwd()never actually returned.
Should address before merge
1. No test for a bug fix. Per AGENTS.md (tests required for bug fixes), this needs coverage, and the pure logic here is trivially testable — apps/desktop/src/store/session.test.ts already exercises store helpers like mergeSessionPage/sessionPinId, so workspaceCwdForNewSession() (configured > remembered > live precedence) belongs right next to them. isPackagedInstallPath() / sanitizeWorkspaceCwd() are similarly unit-testable. (Note: the automated review above states "Test added for the config-driven project directory" — there is no test file in this diff; the checklist box is also unchecked.)
2. applyConfiguredDefaultProjectDir() mutates the live workspace of the active session. It calls setCurrentCwd(configured) unconditionally, and it runs both on the Settings panel mount (sessions-settings.tsx get-effect) and on pick. If a chat is already active in a different directory, this retargets the displayed $currentCwd (and persists it to localStorage) even though the running session's backend cwd is unchanged — and it contradicts your own new toast, "start a new chat (Ctrl/⌘+N) for it to take effect." Suggest splitting concerns: always update the cached configuredDefaultProjectDir, but only setCurrentCwd(...) when there's no active session (fresh draft). That keeps the cache correct for the next new chat without visually rewriting a live session's cwd.
Otherwise clean and well-scoped.
Add workspace cwd precedence tests, extract isPackagedInstallPath for platform test coverage, and stop rewriting live $currentCwd when a session is already active (cache-only until the next new chat). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed the review feedback in a379576: 1. Tests added
2. Active session no longer gets its live cwd rewritten
Let me know if anything else should change before merge. |
OutThisLife
left a comment
There was a problem hiding this comment.
Re-review — both points resolved in a379576f
1. Tests ✅ — workspaceCwdForNewSession() precedence (configured > remembered > live) + the active-session-guard case in session.test.ts, and isPackagedInstallPath() extracted into a DI'd pure module (workspace-cwd.cjs) with workspace-cwd.test.cjs wired into test:desktop:platforms. Bonus: main.cjs now delegates instead of duplicating the loop, and the helper is testable without Electron.
2. Live cwd no longer clobbered ✅ — both applyConfiguredDefaultProjectDir() and ensureDefaultWorkspaceCwd() gate setCurrentCwd(...) on !$activeSessionId.get() via the seedLiveCwd helper; the cache update stays unconditional so the next new chat still honors the default. Matches the toast's contract.
Clean, well-scoped, and tested. LGTM.
…esearch#43234) * fix(desktop): honor default project directory for new sessions The Settings picker persisted project-dir.json but the renderer kept seeding new chats from sticky localStorage home. Prefer the configured default on boot and session.create, pin TERMINAL_CWD at backend spawn, and reject packaged install-dir paths that regressed after NousResearch#37536. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(desktop): address review on default project dir PR Add workspace cwd precedence tests, extract isPackagedInstallPath for platform test coverage, and stop rewriting live $currentCwd when a session is already active (cache-only until the next new chat). Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
What does this PR do?
Fixes a regression where Settings → Archived Chats → Default project directory saved correctly in the main process (
project-dir.json) but new desktop sessions still started in the user's home directory.The renderer was seeding
$currentCwdfrom stickylocalStorage(hermes.desktop.workspace-cwd, PR #37586) and never preferred the configured default when callingsession.create. Restarting the gateway could not help because the cwd was never passed from the picker choice.This PR wires the configured default through boot, new-chat draft, and
session.create; pinsTERMINAL_CWDon backend spawn to matchresolveHermesCwd(); and rejects packaged install-dir paths (e.g.win-unpacked) that could slip back in viaINIT_CWDor remembered workspace paths (PR #37536 item 16 follow-up).Related Issue
Fixes #
Type of Change
Changes Made
apps/desktop/electron/main.cjs—isPackagedInstallPath,sanitizeWorkspaceCwd, skipINIT_CWDwhen packaged, setTERMINAL_CWDon backend spawn,hermes:workspace:sanitizeIPCapps/desktop/electron/preload.cjs— exposesanitizeWorkspaceCwdapps/desktop/src/store/session.ts— cache configured default,ensureDefaultWorkspaceCwd,workspaceCwdForNewSessionapps/desktop/src/app/gateway/hooks/use-gateway-boot.ts— seed workspace on bootapps/desktop/src/app/session/hooks/use-session-actions.ts— new chats use configured defaultapps/desktop/src/app/settings/sessions-settings.tsx— apply picker choice to live workspace immediatelyapps/desktop/src/global.d.ts,apps/desktop/src/i18n/en.ts— bridge/types + clearer success toastHow to Test
Ctrl/⌘+N) and send: "What directory are you working in?"win-unpacked/.appbundle).Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AFor New Skills
N/A — delete section.
Screenshots / Logs
N/A — renderer + Electron IPC change; verify via steps above.