Two findings from the openchamber-bot review at 478f1e9c:
1. Pending selections silently swallowed the Open-in-Finder pick
(prev #4). handleOpenInFinder flows into the batch-first branch of
finalizeSelection, so with checkboxes ticked the OS pick was
ignored and the selections were added instead. Clear selectedPaths
before finalizeSelection so the Finder-sourced target is honored.
2. Within-batch dedup was missing in the VS Code branch (nit from
the previous review). A path repeated within one batch hit
addWorkspaceFolder twice. Mirror the non-VS Code contract with
a seen Set; add a regression test asserting the host is called
once per unique path.
Two non-blockers from the openchamber-bot review at b2ab4af15:
1. VS Code batch add reported a misleading failure. addProjects
returned [] unconditionally for the VS Code runtime because
addWorkspaceFolder is reached only by addProject. Iterate addProject
per path so valid selections succeed and the host is called once
per selection. addProjects is now async (returns Promise<ProjectEntry[]>);
the call site in DirectoryExplorerDialog awaits it; existing
tests in useProjectsStore.test.ts updated to await.
2. Turkish locale (tr.ts) was missing the two new keys that other
11 dictionaries received: actions.addSelected and browse.selectForAdd.
Add both with real Turkish translations: "Seçilenleri ekle" and
"Eklemek için seç".
Two blockers from the openchamber-bot review:
1. Space swallowed in the path input. The handler at
handleKeyDown was calling preventDefault on every Space,
turning paths with spaces into no-op strokes and toggling the
highlighted row instead. Gate the toggle on hasTrailingPathSeparator
(query) so Space is a literal character when the user is typing
a path or filter and only acts as a selection toggle when they
have navigated into a directory.
2. Batch path was unreachable when the filter had no exact match.
With checkboxes ticked and a typed filter that has no exact match,
shouldCreateTarget evaluated true and the primary action (Add
selected) called createDirectory for the typed text instead of
adding the selections. Move the batch branch above
shouldCreateSelection so explicit selections always win over the
single-target create path. Drop the trailing else-if (now
unreachable) which also lost { asProject: true }.
The variable was referenced at line 484 (the single-target create branch)
but never declared, so every non-clone add path (Add button, Cmd+Enter,
Open in Finder, mobile add) threw a ReferenceError that surfaced as a
'Failed to select directory' toast. Add the missing local declaration
matching the condition the original review intended: !isCloneMode &&
shouldCreateTarget && the target path equals the user-typed path.
The checkbox button calls togglePathSelection() on click but does not stop
event propagation, so the click bubbles to the parent row's onClick which
calls executeRow() -> browseToEntry(). With a mouse, every checkbox click
would (a) toggle the selection, (b) navigate into the directory, and (c)
the navigation effect would clear selectedPaths. The primary interaction
of the multi-select feature was unusable.
The existing handleQuickAdd helper avoids this exact bug by calling
event.stopPropagation() inside its onClick handler. Apply the same
pattern to the new togglePathSelection onClick.
Refs openchamber-bot review on #2877.
The SDK client fetch wrapper now applies a 30s timeout to non-streaming
reads. Without it, a socket that neither resolves nor rejects keeps the
directory bootstrap concurrency slot busy forever and the UI stays on
"loading sessions". Long-lived streams (POST prompts, the /event SSE)
are explicitly excluded so they are not cut off mid-flight.
The normalized "request timed out" error is added to the retry
allowlist alongside undici's "terminated" (the exact failure observed
in #2470 when undici tears down a half-open upstream connection); both
are transient while the managed OpenCode process restarts. Caller-
initiated aborts keep their original error shape so a user-cancelled
request is not retried.
Tests cover: GET timeout fires after the bound, POST is not timed out,
/event SSE is not timed out, caller abort wins, AbortError is not
retried, and the SDK normalized error is retried 3x.
`getWorktrees` logs a warn-level line every time the managed OpenCode
process or any other caller passes a directory that is not inside a
git repository. The OpenChamber desktop main.log fills with hundreds
of these "Failed to list worktrees, returning empty list: fatal: not
a git repository ..." entries over a normal session.
The empty-list fallback is already correct (worktrees are an optional
feature), but the warning is noise that hides real git failures. Use
the existing `isNotGitRepositoryError` helper to suppress the warn
specifically for the "not a git repository" case and keep the
warning for genuine failures (lock contention, permission errors,
corrupt repos, etc.).
When switching sessions, the previous session's viewport anchor save
was deferred via setTimeout(..., 0). This races with the new session's
restoreSnapshot effect: the timer can fire after React has flushed
the new session's render and before the restore effect runs, leaving
the saved anchor and the restored scroll position fighting over the
same viewport store entry. The save reads messages (can be expensive)
on the same tick as the new session's skeleton render.
Replace setTimeout(..., 0) with queueMicrotask() so the save runs
immediately after the current synchronous call stack and before the
next macrotask / paint. This guarantees the save completes before the
new session's restoreSnapshot effect fires.
Add a bail check: if the user switched sessions again between the
microtask scheduling and execution (rapid switching), the save is
now stale. Comparing the captured newId to the current currentSessionId
at microtask runtime avoids clobbering the in-flight session's anchor
with data from a session that is no longer "previous".
This is the queueMicrotask + bail change acknowledged as 'great' in
the review of #1675, extracted as a focused single-file PR.
herjarsa