Skip to content

feat(desktop): attach dialogs remember the last-used directory - #93076

Open
ClintonEmok wants to merge 2 commits into
NousResearch:mainfrom
ClintonEmok:feat/picker-remembers-last-dir
Open

feat(desktop): attach dialogs remember the last-used directory#93076
ClintonEmok wants to merge 2 commits into
NousResearch:mainfrom
ClintonEmok:feat/picker-remembers-last-dir

Conversation

@ClintonEmok

Copy link
Copy Markdown
Contributor

What does this PR do?

The native open dialogs (composer Files/Images/Folder, plugin source, etc.) started from scratch on every attach — a user pulling a dozen screenshots from the same folder re-navigated there for each one (#92925).

The main process now records the parent directory of each successful pick and offers it as the dialog's defaultPath when the caller has no opinion of its own. Precedence ladder:

  1. Explicit defaultPath from the caller (the composer passes its cwd; [Feature]: Desktop — attach file dialog should default to Downloads when the session has no workspace #91074's Downloads default applies when detached) — it expresses current intent
  2. The remembered last-used directory, only when it still exists on disk
  3. undefined → OS default

Electron is authoritative for machine-side facts, so the memory lives in the main process (userData/picker-state.json), not in renderer state.

Related Issue

Fixes #92925
(Complements #91074 rather than duplicating it: that PR is the initial default, this remembers what the user actually chose, which beats any static default after the first attach.)

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)

Changes Made

  • apps/desktop/electron/picker-state.ts (new) — pure helpers: nextPickerDefaultPath (precedence policy), dirToRemember (parent-of-first-pick rule), atomic read/write of picker-state.json; corrupt file reads as "nothing remembered"
  • apps/desktop/electron/main.tshermes:selectPaths consults the remembered directory when no explicit default was given, and records the pick on success; canceled dialogs change nothing
  • apps/desktop/electron/picker-state.test.ts — 8 cases covering the precedence ladder, the remember rule, round-trip persistence, and corrupt-state fallback

How to Test

  1. cd apps/desktop && npx vitest run electron/picker-state.test.ts — 8 passed
  2. Attach a file from a folder other than Downloads, then detach the app from any workspace so the composer sends no defaultPath: the next attach dialog opens in the folder you just used
  3. With a workspace active (composer cwd present), the dialog still opens at the cwd — explicit intent wins
  4. Delete the remembered folder; the next dialog falls back to the OS default instead of erroring

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run targeted tests (npx vitest run --project electron: 1606 passed; tsc clean)
  • I've added tests for my changes
  • I've tested on my platform: macOS 26.5 (Apple Silicon)

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A (state lives in userData, not config.yaml)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — path joins via node:path, atomic write via temp+rename
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

The native open dialogs started from scratch on every attach — a user
pulling a dozen screenshots from the same folder re-navigated there for
each one (NousResearch#92925).

The main process now records the parent directory of each successful
pick in userData/picker-state.json (machine-side fact, Electron's
authority) and offers it as defaultPath when the caller has no opinion
of its own. Precedence ladder: explicit defaultPath (composer cwd,
NousResearch#91074's Downloads default) > remembered last dir when it still exists
> OS default. Pure helpers in electron/picker-state.ts, covered by
unit tests for the precedence policy, the remember rule, and the
corrupt-file fallback.

Fixes NousResearch#92925
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) labels Aug 23, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Clean, well-tested module: pure precedence function, atomic tmp+rename persistence, corrupt-snapshot fail-closed, and good negative tests. Two things to resolve:

  1. Duplication with fix(gateway): ws-orphan reaper leaves the canonical Bot Chat resumable #93077: this diff appears byte-identical to the picker-state portion of PR fix(gateway): ws-orphan reaper leaves the canonical Bot Chat resumable #93077 (same blob hashes for picker-state.ts and main.tsc12ae23168a5 / a28f98b346f4). Whichever lands second will conflict; if the intent is to split that PR apart, closing/superseding the other half explicitly would save reviewers double work.

  2. apps/desktop/electron/main.ts:14084-14088 — the last-used directory is written on every successful pick, including picks where the caller supplied an explicit composer-cwd defaultPath. That's arguably correct ("memory follows actual usage"), but it's a policy choice worth stating in the docstring, because the alternative (only remember unprompted picks) produces different behavior for users who mostly accept suggested folders.

Minor: picker-state.ts:190-201 — the temp file name includes pid + random suffix, so concurrent windows can't rename over each other's in-flight file, but two near-simultaneous successful picks still race last-write-wins on content. Fine for a convenience memory; just noting it's not serialized.

The precedence tests (explicit wins / remembered ignored when gone / OS default) cover exactly the right matrix.

The electron/main.ts import block must keep ./picker-state ahead of
./wsl-clipboard-image per perfectionist/sort-imports; the CI run on
this PR failed JS & TS checks on exactly that.
@ClintonEmok

Copy link
Copy Markdown
Contributor Author

Both review points addressed:

  1. Import ordering — the CI failure was perfectionist/sort-imports on electron/main.ts (./picker-state must precede ./wsl-clipboard-image). Fixed in the new head commit; JS & TS checks should go green.

  2. Docstring policy note — you're right that "memory follows actual usage" is a policy choice. Documented it explicitly: every successful pick updates the memory, including ones that accepted a suggested default, because the alternative (remember only unprompted picks) would freeze the memory on whatever folder the user first strayed to. The last-write-wins concurrency note is fair and left as-is — this is a convenience memory, not a source of truth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Desktop — attach file dialog should remember the last-used directory

3 participants