perf: reduce re-renders, fix mobile keyboard handling, add chunk load recovery, and improve PATH management (#1028)
* fix: exclude file content from reverted prompt text Revert and fork now restore only the user's original prompt, not server-injected file content Uses existing isSyntheticPart helper for type-safe filtering * fix: keep scrollbar visible when hovering over thumb * fix: prevent ESC abort from triggering when terminal is focused * fix: pass directory to permission/question reply calls so approvals actually resolve * fix: default model selection not responding after Base UI migration * fix: prevent modal content from shifting and clipping footer buttons * fix: improve session switching performance and add sub-agent export with prompt collapse Defer viewport anchor saving to eliminate ~800ms UI freeze when switching sessions Add export dialog to include sub-agent tasks recursively in markdown export Add collapse chevron button for expanded user prompts in sticky header * fix: resolve sidebar scroll and TDZ crash in session sidebar * perf: reduce CPU overhead and re-renders across chat, layout, and settings * fix: position collapse button at top of message and prevent ESC abort in terminal * fix: position collapse button at top and add padding only when expanded * refactor: extract shared PATH utilities and mobile keyboard hook * refactor: import shared path-utils in electron, use module-level style constants - Electron now imports pathLooksUserConfigured/mergePathValues from shared path-utils.js instead of inline duplication - ToolPart collapsedCustomStyle moved from useMemo([]) to module const * fix: resolve remaining merge conflicts and type errors - Remove duplicate variable declarations in SessionNodeItem - Remove orphaned export callback body from conflict resolution - Fix HelpDialog description -> descriptionKey (i18n rename) * fix: resolve type-check and lint errors in session-actions.test.ts - Added missing bun:test type declarations (beforeEach, mock, mock.module) - Removed unused State import - Replaced 'as any' casts with proper OpencodeClient and ChildStoreManager types - Added eslint-disable for unused _ parameter in mock function * fix PR 1028 export and PATH edge cases * fix startup retry exhaustion state * remove opencode package lock change * fix sub-session rename cancellation --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
Bohdan Triapitsyn
parent
632e6cc97b
commit
4523e9c486
@@ -2,6 +2,7 @@ import { spawnSync } from 'node:child_process';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { pathLooksUserConfigured, mergePathValues } from './path-utils.js';
|
||||
|
||||
export const createOpenCodeEnvRuntime = (deps) => {
|
||||
const {
|
||||
@@ -162,21 +163,6 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
return null;
|
||||
};
|
||||
|
||||
const pathLooksUserConfigured = (value) => {
|
||||
if (typeof value !== 'string' || !value) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const home = os.homedir();
|
||||
return value.split(path.delimiter).some((segment) => (
|
||||
segment.startsWith(home + path.sep)
|
||||
|| segment === home
|
||||
|| segment.startsWith('/opt/homebrew/')
|
||||
|| segment.startsWith('/opt/pkg/')
|
||||
|| segment.startsWith('/opt/pmk/')
|
||||
));
|
||||
};
|
||||
|
||||
const applyLoginShellEnvSnapshot = () => {
|
||||
const snapshot = getLoginShellEnvSnapshot();
|
||||
if (!snapshot) {
|
||||
@@ -197,8 +183,9 @@ export const createOpenCodeEnvRuntime = (deps) => {
|
||||
|
||||
const currentPath = process.env.PATH || '';
|
||||
const shellPath = snapshot.PATH || '';
|
||||
if (!pathLooksUserConfigured(currentPath) && shellPath) {
|
||||
process.env.PATH = shellPath;
|
||||
const home = os.homedir();
|
||||
if (!pathLooksUserConfigured(currentPath, home, path.delimiter) && shellPath) {
|
||||
process.env.PATH = mergePathValues(shellPath, currentPath, path.delimiter);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -116,4 +116,51 @@ describe('OpenCode lifecycle', () => {
|
||||
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it('falls back to buildAugmentedPath when buildManagedOpenCodePath is not provided', async () => {
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
const child = createMockChild();
|
||||
spawnMock.mockImplementationOnce(() => {
|
||||
queueMicrotask(() => {
|
||||
child.stdout.emit('data', 'opencode server listening on http://127.0.0.1:45678\n');
|
||||
});
|
||||
return child;
|
||||
});
|
||||
|
||||
const runtime = createRuntime({
|
||||
buildManagedOpenCodePath: undefined,
|
||||
buildAugmentedPath: vi.fn(() => '/home/user/.cargo/bin:/usr/local/bin'),
|
||||
});
|
||||
const server = await runtime.startOpenCode();
|
||||
const [, , options] = spawnMock.mock.calls[0];
|
||||
|
||||
expect(options.env.PATH).toBe('/home/user/.cargo/bin:/usr/local/bin');
|
||||
|
||||
await server.close();
|
||||
});
|
||||
|
||||
it('falls back to process.env.PATH when neither build function is provided', async () => {
|
||||
delete process.env.OPENCODE_BINARY;
|
||||
const originalPath = process.env.PATH;
|
||||
process.env.PATH = '/usr/bin:/bin';
|
||||
const child = createMockChild();
|
||||
spawnMock.mockImplementationOnce(() => {
|
||||
queueMicrotask(() => {
|
||||
child.stdout.emit('data', 'opencode server listening on http://127.0.0.1:45678\n');
|
||||
});
|
||||
return child;
|
||||
});
|
||||
|
||||
const runtime = createRuntime({
|
||||
buildManagedOpenCodePath: undefined,
|
||||
buildAugmentedPath: undefined,
|
||||
});
|
||||
const server = await runtime.startOpenCode();
|
||||
const [, , options] = spawnMock.mock.calls[0];
|
||||
|
||||
expect(options.env.PATH).toBe('/usr/bin:/bin');
|
||||
process.env.PATH = originalPath;
|
||||
|
||||
await server.close();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
/**
|
||||
* Shared PATH heuristics and merge utilities for server and Electron runtimes.
|
||||
*
|
||||
* The heuristic decides whether the current process.env.PATH looks like it was
|
||||
* configured by the user (or their session manager) vs. a minimal system default.
|
||||
* When the PATH looks user-configured we keep it; otherwise we prefer the login
|
||||
* shell PATH which typically has the full toolchain.
|
||||
*/
|
||||
|
||||
const TOOLCHAIN_SEGMENTS = [
|
||||
'/opt/homebrew/',
|
||||
'/opt/pkg/',
|
||||
'/opt/pmk/',
|
||||
'/snap/',
|
||||
];
|
||||
|
||||
const TOOLCHAIN_BASENAMES = new Set([
|
||||
'.cargo',
|
||||
'.bun',
|
||||
'.nvm',
|
||||
'.pyenv',
|
||||
'.rbenv',
|
||||
'.sdkman',
|
||||
'.asdf',
|
||||
'.volta',
|
||||
'.fnm',
|
||||
'.local',
|
||||
'.opencode',
|
||||
'node_modules',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Returns true when `value` (a PATH string) contains at least one segment that
|
||||
* suggests the PATH was configured by the user or their session manager rather
|
||||
* than being a bare system default.
|
||||
*
|
||||
* @param {string} value - The PATH string to inspect.
|
||||
* @param {string} home - The user's home directory (os.homedir()).
|
||||
* @param {string} delim - The PATH delimiter (':' on POSIX, ';' on Windows).
|
||||
*/
|
||||
export function pathLooksUserConfigured(value, home, delim) {
|
||||
if (typeof value !== 'string' || !value) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const normalizedHome = typeof home === 'string' ? home.replaceAll('\\', '/') : '';
|
||||
const homeWithSep = normalizedHome ? normalizedHome + '/' : '';
|
||||
|
||||
return value.split(delim).some((segment) => {
|
||||
if (!segment) return false;
|
||||
const normalizedSegment = segment.replaceAll('\\', '/');
|
||||
|
||||
// Any path under the user's home directory.
|
||||
if (normalizedHome && (normalizedSegment === normalizedHome || normalizedSegment.startsWith(homeWithSep))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Well-known package-manager / toolchain prefixes.
|
||||
if (TOOLCHAIN_SEGMENTS.some((prefix) => normalizedSegment.startsWith(prefix))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Well-known dot-directories inside home (e.g. ~/.cargo/bin).
|
||||
const parts = normalizedSegment.split('/').filter(Boolean);
|
||||
if (parts.some((part) => TOOLCHAIN_BASENAMES.has(part))) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Merges two PATH strings, deduplicating segments while preserving the order of
|
||||
* `primary` and appending any segments from `fallback` that are not already
|
||||
* present.
|
||||
*
|
||||
* @param {string} primary - The preferred PATH (e.g. user-configured or login shell).
|
||||
* @param {string} fallback - The secondary PATH to fill gaps from.
|
||||
* @param {string} delim - The PATH delimiter.
|
||||
*/
|
||||
export function mergePathValues(primary, fallback, delim) {
|
||||
const seen = new Set();
|
||||
const result = [];
|
||||
|
||||
const addSegments = (value) => {
|
||||
if (typeof value !== 'string' || !value) return;
|
||||
for (const segment of value.split(delim)) {
|
||||
if (segment && !seen.has(segment)) {
|
||||
seen.add(segment);
|
||||
result.push(segment);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
addSegments(primary);
|
||||
addSegments(fallback);
|
||||
|
||||
return result.join(delim);
|
||||
}
|
||||
@@ -0,0 +1,71 @@
|
||||
import path from 'node:path';
|
||||
import os from 'node:os';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { pathLooksUserConfigured, mergePathValues } from './path-utils.js';
|
||||
|
||||
const home = os.homedir();
|
||||
const delim = path.delimiter;
|
||||
|
||||
describe('pathLooksUserConfigured', () => {
|
||||
it('returns false for empty or non-string values', () => {
|
||||
expect(pathLooksUserConfigured('', home, delim)).toBe(false);
|
||||
expect(pathLooksUserConfigured(null, home, delim)).toBe(false);
|
||||
expect(pathLooksUserConfigured(undefined, home, delim)).toBe(false);
|
||||
expect(pathLooksUserConfigured(42, home, delim)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for minimal system PATH', () => {
|
||||
expect(pathLooksUserConfigured('/usr/local/bin:/usr/bin:/bin', home, delim)).toBe(false);
|
||||
});
|
||||
|
||||
it('detects paths under home directory', () => {
|
||||
expect(pathLooksUserConfigured(`${home}/.bun/bin:/usr/bin`, home, delim)).toBe(true);
|
||||
expect(pathLooksUserConfigured(`${home}/.local/bin:/usr/bin`, home, delim)).toBe(true);
|
||||
});
|
||||
|
||||
it('detects home directory itself', () => {
|
||||
expect(pathLooksUserConfigured(`${home}:/usr/bin`, home, delim)).toBe(true);
|
||||
});
|
||||
|
||||
it('detects well-known package manager prefixes', () => {
|
||||
expect(pathLooksUserConfigured('/opt/homebrew/bin:/usr/bin', home, delim)).toBe(true);
|
||||
expect(pathLooksUserConfigured('/opt/pkg/bin:/usr/bin', home, delim)).toBe(true);
|
||||
expect(pathLooksUserConfigured('/snap/bin:/usr/bin', home, delim)).toBe(true);
|
||||
});
|
||||
|
||||
it('detects well-known dot-directory basenames', () => {
|
||||
expect(pathLooksUserConfigured('/some/path/.cargo/bin:/usr/bin', home, delim)).toBe(true);
|
||||
expect(pathLooksUserConfigured('/some/path/.nvm/versions/node/v20/bin:/usr/bin', home, delim)).toBe(true);
|
||||
expect(pathLooksUserConfigured('/some/path/.pyenv/shims:/usr/bin', home, delim)).toBe(true);
|
||||
expect(pathLooksUserConfigured('/some/path/.opencode/bin:/usr/bin', home, delim)).toBe(true);
|
||||
});
|
||||
|
||||
it('detects Windows home and toolchain paths', () => {
|
||||
const windowsHome = 'C:\\Users\\agent';
|
||||
expect(pathLooksUserConfigured('C:\\Users\\agent\\.bun\\bin;C:\\Windows\\System32', windowsHome, ';')).toBe(true);
|
||||
expect(pathLooksUserConfigured('C:\\tools\\.cargo\\bin;C:\\Windows\\System32', windowsHome, ';')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('mergePathValues', () => {
|
||||
it('returns empty string for empty inputs', () => {
|
||||
expect(mergePathValues('', '', delim)).toBe('');
|
||||
});
|
||||
|
||||
it('returns primary when fallback is empty', () => {
|
||||
expect(mergePathValues('/a:/b', '', delim)).toBe('/a:/b');
|
||||
});
|
||||
|
||||
it('returns fallback when primary is empty', () => {
|
||||
expect(mergePathValues('', '/a:/b', delim)).toBe('/a:/b');
|
||||
});
|
||||
|
||||
it('deduplicates segments, preserving primary order', () => {
|
||||
expect(mergePathValues('/a:/b:/c', '/b:/d:/a', delim)).toBe('/a:/b:/c:/d');
|
||||
});
|
||||
|
||||
it('appends all fallback segments when no overlap', () => {
|
||||
expect(mergePathValues('/a:/b', '/c:/d', delim)).toBe('/a:/b:/c:/d');
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import { registerOpenCodeProxy } from './proxy.js';
|
||||
import { pathLooksUserConfigured, mergePathValues } from './path-utils.js';
|
||||
|
||||
export const createServerUtilsRuntime = (dependencies) => {
|
||||
const {
|
||||
@@ -45,21 +46,6 @@ export const createServerUtilsRuntime = (dependencies) => {
|
||||
clearLastOpenCodeError();
|
||||
};
|
||||
|
||||
const pathLooksUserConfigured = (value) => {
|
||||
if (typeof value !== 'string' || !value) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const home = os.homedir();
|
||||
return value.split(path.delimiter).some((segment) => (
|
||||
segment.startsWith(home + path.sep)
|
||||
|| segment === home
|
||||
|| segment.startsWith('/opt/homebrew/')
|
||||
|| segment.startsWith('/opt/pkg/')
|
||||
|| segment.startsWith('/opt/pmk/')
|
||||
));
|
||||
};
|
||||
|
||||
const waitForOpenCodePort = async (timeoutMs = 15000) => {
|
||||
if (getOpenCodePort() !== null) {
|
||||
return getOpenCodePort();
|
||||
@@ -77,40 +63,26 @@ export const createServerUtilsRuntime = (dependencies) => {
|
||||
};
|
||||
|
||||
const buildAugmentedPath = () => {
|
||||
const home = os.homedir();
|
||||
const currentPath = process.env.PATH || '';
|
||||
const loginShellPath = getLoginShellPath();
|
||||
const currentPathLooksUserConfigured = pathLooksUserConfigured(currentPath);
|
||||
const home = os.homedir();
|
||||
const currentPathLooksUserConfigured = pathLooksUserConfigured(currentPath, home, path.delimiter);
|
||||
const primaryPath = currentPathLooksUserConfigured ? currentPath : loginShellPath;
|
||||
const fallbackPath = currentPathLooksUserConfigured ? loginShellPath : currentPath;
|
||||
const seen = new Set();
|
||||
const augmented = [];
|
||||
|
||||
const addSegments = (value) => {
|
||||
if (typeof value !== 'string' || !value) {
|
||||
return;
|
||||
}
|
||||
for (const segment of value.split(path.delimiter)) {
|
||||
if (segment && !seen.has(segment)) {
|
||||
seen.add(segment);
|
||||
augmented.push(segment);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
addSegments(primaryPath);
|
||||
addSegments(fallbackPath);
|
||||
|
||||
return augmented.join(path.delimiter);
|
||||
return mergePathValues(primaryPath, fallbackPath, path.delimiter);
|
||||
};
|
||||
|
||||
const buildManagedOpenCodePath = () => {
|
||||
const currentPath = process.env.PATH || '';
|
||||
if (pathLooksUserConfigured(currentPath)) {
|
||||
const loginShellPath = getLoginShellPath();
|
||||
const home = os.homedir();
|
||||
|
||||
if (pathLooksUserConfigured(currentPath, home, path.delimiter)) {
|
||||
return currentPath;
|
||||
}
|
||||
|
||||
return getLoginShellPath() || currentPath;
|
||||
return mergePathValues(loginShellPath || '', currentPath, path.delimiter);
|
||||
};
|
||||
|
||||
const parseSseDataPayload = (block) => {
|
||||
|
||||
@@ -66,7 +66,15 @@ describe('server utils runtime', () => {
|
||||
|
||||
const runtime = createRuntime(loginShellPath);
|
||||
|
||||
expect(runtime.buildManagedOpenCodePath()).toBe(loginShellPath);
|
||||
// Should prefer login shell PATH but merge in any process entries not already present.
|
||||
expect(runtime.buildManagedOpenCodePath()).toBe([
|
||||
path.join(home, '.opencode', 'bin'),
|
||||
path.join(home, '.bun', 'bin'),
|
||||
'/opt/homebrew/bin',
|
||||
'/usr/bin',
|
||||
'/usr/local/bin',
|
||||
'/bin',
|
||||
].join(path.delimiter));
|
||||
});
|
||||
|
||||
it('preserves user-configured process PATH order before appending shell-only entries', () => {
|
||||
|
||||
Reference in New Issue
Block a user