-
Notifications
You must be signed in to change notification settings - Fork 305
feat(desktop): add a default working directory #2998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import assert from 'node:assert/strict'; | ||
| import { test } from 'node:test'; | ||
| import { registerRuntimeHostSettingsIpc } from '../runtime-host-settings-ipc-main.js'; | ||
|
|
||
| test('returns the directory selected by the main process', async () => { | ||
| const handlers = new Map<string, (...args: unknown[]) => unknown>(); | ||
| registerRuntimeHostSettingsIpc({ | ||
| ipcMain: { | ||
| handle(channel, handler) { | ||
| handlers.set(channel, handler as (...args: unknown[]) => unknown); | ||
| }, | ||
| }, | ||
| client: {} as never, | ||
| settingsStore: {} as never, | ||
| applyClientSettings: async () => undefined, | ||
| chooseDefaultWorkingDirectory: async () => '/Users/example/agent', | ||
| }); | ||
|
|
||
| const choose = handlers.get('settings:chooseDefaultWorkingDirectory'); | ||
| assert.ok(choose); | ||
| assert.equal(await choose({}), '/Users/example/agent'); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { randomUUID } from 'node:crypto'; | ||
| import { readFile, rename, rm, stat, writeFile } from 'node:fs/promises'; | ||
| import { resolve } from 'node:path'; | ||
| import { resolveProjectRoot } from '@maka/runtime/system-prompt/project-context'; | ||
|
|
||
| export interface CurrentProjectSelection { | ||
|
|
@@ -23,6 +24,7 @@ export interface ProjectRootControllerDeps { | |
| readonly rootId: string; | ||
| readonly preferenceFile: string; | ||
| readonly fallbackRoots: () => string[]; | ||
| readonly defaultWorkingDirectory?: () => Promise<string | undefined>; | ||
| } | ||
|
|
||
| interface ProjectPreferenceFile { | ||
|
|
@@ -39,8 +41,10 @@ export function createProjectRootController( | |
| const initialSelection = loadInitialSelection(deps); | ||
|
|
||
| async function currentSelection(): Promise<CurrentProjectSelection> { | ||
| if (selectedProject) return selectedProject; | ||
| return (selectedProject = await initialSelection); | ||
| const selection = selectedProject ?? (selectedProject = await initialSelection); | ||
| if (typeof selection.projectId === 'string') return selection; | ||
| const path = await resolveUnassociatedRoot(deps); | ||
| return (selectedProject = { ...selection, path }); | ||
|
Comment on lines
+46
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Concurrent selection gets overwritten While currentSelection() awaits dynamic default-directory resolution, setSelection() can install an explicit Project, but line 47 then unconditionally replaces it with the stale unassociated selection. A concurrent Project add or selection can therefore appear successful and be persisted while the active session continues without that Project. Agent Prompt
|
||
| } | ||
|
|
||
| async function current(): Promise<string> { | ||
|
|
@@ -73,11 +77,21 @@ export function createProjectRootController( | |
| async function loadInitialSelection( | ||
| deps: ProjectRootControllerDeps, | ||
| ): Promise<CurrentProjectSelection> { | ||
| const fallbackPath = await resolveProjectRoot(deps.fallbackRoots()); | ||
| const fallbackPath = await resolveUnassociatedRoot(deps); | ||
| const preference = await readPreference(deps.preferenceFile, deps.rootId); | ||
| return { projectId: preference, path: fallbackPath }; | ||
| } | ||
|
|
||
| async function resolveUnassociatedRoot(deps: ProjectRootControllerDeps): Promise<string> { | ||
| const configured = await deps.defaultWorkingDirectory?.(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 — Keep the optional default directory from blocking Project recovery. |
||
| if (configured) { | ||
| const path = resolve(configured); | ||
| const info = await stat(path).catch(() => undefined); | ||
| if (info?.isDirectory()) return path; | ||
| } | ||
| return resolveProjectRoot(deps.fallbackRoots()); | ||
| } | ||
|
|
||
| async function persistSelection( | ||
| deps: ProjectRootControllerDeps, | ||
| projectId: string | null, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Do not let initial resolution overwrite a newer explicit selection.
setSelection()updatesselectedProjectsynchronously, but if thisawait initialSelectionis already pending, its continuation still assigns the stale unassociated selection here after the explicit Project selection has been persisted. A caller can therefore successfully select a Project while the active controller silently falls back to the default directory. Re-checkselectedProjectafter the await (and likewise after the later default-root await), or fence both continuations with a generation/ticket; add a deferred-resolution regression that callssetSelection()before each await settles.