Compare commits

..

2 Commits

Author SHA1 Message Date
Simos Mikelatos
80ce5b8313 Merge branch 'main' into fix/shell-user-npm-path-priority 2026-06-24 11:10:47 +02:00
Haileyesus
9a33426eed fix(shell): prioritize user npm binaries
Interactive shells could resolve bundled or system CLIs before user-installed npm binaries.

Move existing user npm global directories to the front of PATH while preserving all other entries.
2026-06-22 15:27:13 +03:00
6 changed files with 78 additions and 68 deletions

View File

@@ -171,6 +171,62 @@ function buildShellCommand(
return command; return command;
} }
function readEnvValue(env: NodeJS.ProcessEnv, key: string): string | undefined {
const resolvedKey = Object.keys(env).find((envKey) => envKey.toLowerCase() === key.toLowerCase());
return resolvedKey ? env[resolvedKey] : undefined;
}
function getPathEnvKey(env: NodeJS.ProcessEnv): string {
return Object.keys(env).find((key) => key.toLowerCase() === 'path') || 'PATH';
}
function prioritizeUserNpmGlobalBin(env: NodeJS.ProcessEnv): { key: string; value: string | undefined } {
const pathKey = getPathEnvKey(env);
const currentPath = env[pathKey];
if (!currentPath) {
return { key: pathKey, value: currentPath };
}
const delimiter = path.delimiter;
const pathEntries = currentPath.split(delimiter).filter(Boolean);
const npmPrefix = readEnvValue(env, 'npm_config_prefix');
const appData = readEnvValue(env, 'APPDATA');
const candidates = [
npmPrefix || '',
npmPrefix ? path.join(npmPrefix, 'bin') : '',
appData ? path.join(appData, 'npm') : '',
path.join(os.homedir(), 'AppData', 'Roaming', 'npm'),
path.join(os.homedir(), '.npm-global', 'bin'),
].filter(Boolean);
const normalizedPathEntries = pathEntries.map((entry) => os.platform() === 'win32' ? entry.toLowerCase() : entry);
const preferredEntries = candidates.filter((candidate, index) => {
const normalizedCandidate = os.platform() === 'win32' ? candidate.toLowerCase() : candidate;
return (
candidates.indexOf(candidate) === index &&
normalizedPathEntries.includes(normalizedCandidate)
);
});
if (preferredEntries.length === 0) {
return { key: pathKey, value: currentPath };
}
const normalizedPreferredEntries = preferredEntries.map((entry) =>
os.platform() === 'win32' ? entry.toLowerCase() : entry
);
const value = [
...preferredEntries,
...pathEntries.filter((entry) => {
const normalizedEntry = os.platform() === 'win32' ? entry.toLowerCase() : entry;
return !normalizedPreferredEntries.includes(normalizedEntry);
}),
].join(delimiter);
return { key: pathKey, value };
}
/** /**
* Handles websocket connections used by the standalone shell terminal UI. * Handles websocket connections used by the standalone shell terminal UI.
*/ */
@@ -284,6 +340,7 @@ export function handleShellConnection(
os.platform() === 'win32' ? ['-Command', shellCommand] : ['-c', shellCommand]; os.platform() === 'win32' ? ['-Command', shellCommand] : ['-c', shellCommand];
const termCols = readNumber(data.cols, 80); const termCols = readNumber(data.cols, 80);
const termRows = readNumber(data.rows, 24); const termRows = readNumber(data.rows, 24);
const prioritizedPath = prioritizeUserNpmGlobalBin(process.env);
shellProcess = pty.spawn(shell, shellArgs, { shellProcess = pty.spawn(shell, shellArgs, {
name: 'xterm-256color', name: 'xterm-256color',
@@ -292,6 +349,7 @@ export function handleShellConnection(
cwd: resolvedProjectPath, cwd: resolvedProjectPath,
env: { env: {
...process.env, ...process.env,
[prioritizedPath.key]: prioritizedPath.value,
TERM: 'xterm-256color', TERM: 'xterm-256color',
COLORTERM: 'truecolor', COLORTERM: 'truecolor',
FORCE_COLOR: '3', FORCE_COLOR: '3',

View File

@@ -114,6 +114,7 @@ export function useChatProviderState({ selectedSession, selectedProject }: UseCh
const [providerModelsLoading, setProviderModelsLoading] = useState(true); const [providerModelsLoading, setProviderModelsLoading] = useState(true);
const [providerModelsRefreshing, setProviderModelsRefreshing] = useState(false); const [providerModelsRefreshing, setProviderModelsRefreshing] = useState(false);
const lastProviderRef = useRef(provider);
const providerModelsRequestIdRef = useRef(0); const providerModelsRequestIdRef = useRef(0);
const setStoredProviderModel = useCallback((targetProvider: LLMProvider, model: string) => { const setStoredProviderModel = useCallback((targetProvider: LLMProvider, model: string) => {
@@ -343,8 +344,14 @@ export function useChatProviderState({ selectedSession, selectedProject }: UseCh
localStorage.setItem('selected-provider', selectedSession.__provider); localStorage.setItem('selected-provider', selectedSession.__provider);
}, [provider, selectedSession]); }, [provider, selectedSession]);
// Permission prompts belong to a session, not to the transient provider useEffect(() => {
// selection that is synchronized after navigation. if (lastProviderRef.current === provider) {
return;
}
setPendingPermissionRequests([]);
lastProviderRef.current = provider;
}, [provider]);
useEffect(() => { useEffect(() => {
setPendingPermissionRequests((previous) => setPendingPermissionRequests((previous) =>
previous.filter((request) => !request.sessionId || request.sessionId === selectedSession?.id), previous.filter((request) => !request.sessionId || request.sessionId === selectedSession?.id),

View File

@@ -1,29 +1,20 @@
import { useEffect, useRef } from 'react'; import { useEffect } from 'react';
import type { Dispatch, MutableRefObject, SetStateAction } from 'react'; import type { Dispatch, MutableRefObject, SetStateAction } from 'react';
import type { ServerEvent } from '../../../contexts/WebSocketContext'; import type { ServerEvent } from '../../../contexts/WebSocketContext';
import { showCompletionTitleIndicator } from '../../../utils/pageTitleNotification'; import { showCompletionTitleIndicator } from '../../../utils/pageTitleNotification';
import { playChatCompletionSound, playNotificationSound } from '../../../utils/notificationSound'; import { playChatCompletionSound } from '../../../utils/notificationSound';
import type { MarkSessionIdle, MarkSessionProcessing } from '../../../hooks/useSessionProtection'; import type { MarkSessionIdle, MarkSessionProcessing } from '../../../hooks/useSessionProtection';
import type { PendingPermissionRequest } from '../types/types'; import type { PendingPermissionRequest } from '../types/types';
import type { ProjectSession, LLMProvider } from '../../../types/app'; import type { ProjectSession, LLMProvider } from '../../../types/app';
import type { SessionStore, NormalizedMessage } from '../../../stores/useSessionStore'; import type { SessionStore, NormalizedMessage } from '../../../stores/useSessionStore';
const isActionablePermissionRequest = (request: { toolName?: unknown } | null | undefined): boolean => {
return request?.toolName !== 'ExitPlanMode' && request?.toolName !== 'exit_plan_mode';
};
const hasActionablePermissionRequests = (requests: Array<{ toolName?: unknown }> | null | undefined): boolean => {
return Array.isArray(requests) && requests.some((request) => isActionablePermissionRequest(request));
};
interface UseChatRealtimeHandlersArgs { interface UseChatRealtimeHandlersArgs {
subscribe: (listener: (event: ServerEvent) => void) => () => void; subscribe: (listener: (event: ServerEvent) => void) => () => void;
provider: LLMProvider; provider: LLMProvider;
selectedSession: ProjectSession | null; selectedSession: ProjectSession | null;
currentSessionId: string | null; currentSessionId: string | null;
setTokenBudget: (budget: Record<string, unknown> | null) => void; setTokenBudget: (budget: Record<string, unknown> | null) => void;
pendingPermissionRequests: PendingPermissionRequest[];
setPendingPermissionRequests: Dispatch<SetStateAction<PendingPermissionRequest[]>>; setPendingPermissionRequests: Dispatch<SetStateAction<PendingPermissionRequest[]>>;
streamTimerRef: MutableRefObject<number | null>; streamTimerRef: MutableRefObject<number | null>;
accumulatedStreamRef: MutableRefObject<string>; accumulatedStreamRef: MutableRefObject<string>;
@@ -61,7 +52,6 @@ export function useChatRealtimeHandlers({
selectedSession, selectedSession,
currentSessionId, currentSessionId,
setTokenBudget, setTokenBudget,
pendingPermissionRequests,
setPendingPermissionRequests, setPendingPermissionRequests,
streamTimerRef, streamTimerRef,
accumulatedStreamRef, accumulatedStreamRef,
@@ -72,29 +62,13 @@ export function useChatRealtimeHandlers({
onWebSocketReconnect, onWebSocketReconnect,
sessionStore, sessionStore,
}: UseChatRealtimeHandlersArgs) { }: UseChatRealtimeHandlersArgs) {
// Session switches can send `chat.subscribe` before this effect has a chance
// to rebind the websocket listener. Read the visible session id from a ref
// so a fast `chat_subscribed` ack is matched against the current view, not
// the previous render's closed-over selection.
const activeViewSessionIdRef = useRef<string | null>(selectedSession?.id || currentSessionId || null);
activeViewSessionIdRef.current = selectedSession?.id || currentSessionId || null;
// Keep the latest pending-permission snapshot available to the websocket
// listener so back-to-back permission events can dedupe and re-arm the
// notification sound before React finishes a rerender.
const pendingPermissionRequestsRef = useRef(pendingPermissionRequests);
useEffect(() => {
pendingPermissionRequestsRef.current = pendingPermissionRequests;
}, [pendingPermissionRequests]);
useEffect(() => { useEffect(() => {
const handleEvent = (msg: ServerEvent) => { const handleEvent = (msg: ServerEvent) => {
if (!msg.kind) { if (!msg.kind) {
return; return;
} }
const activeViewSessionId = activeViewSessionIdRef.current; const activeViewSessionId = selectedSession?.id || currentSessionId || null;
const sid = (typeof msg.sessionId === 'string' && msg.sessionId) || activeViewSessionId; const sid = (typeof msg.sessionId === 'string' && msg.sessionId) || activeViewSessionId;
// Record replay progress for every sequenced live event. // Record replay progress for every sequenced live event.
@@ -127,16 +101,7 @@ export function useChatRealtimeHandlers({
const isViewedSession = sid === activeViewSessionId; const isViewedSession = sid === activeViewSessionId;
if (isViewedSession && Array.isArray(msg.pendingPermissions)) { if (isViewedSession && Array.isArray(msg.pendingPermissions)) {
const nextPendingPermissionRequests = msg.pendingPermissions as PendingPermissionRequest[]; setPendingPermissionRequests(msg.pendingPermissions as PendingPermissionRequest[]);
const hadActionablePermissionRequests = hasActionablePermissionRequests(pendingPermissionRequestsRef.current);
const hasPendingActionablePermissionRequests = hasActionablePermissionRequests(nextPendingPermissionRequests);
pendingPermissionRequestsRef.current = nextPendingPermissionRequests;
setPendingPermissionRequests(nextPendingPermissionRequests);
if (hasPendingActionablePermissionRequests && !hadActionablePermissionRequests) {
void playNotificationSound();
}
} }
return; return;
} }
@@ -238,7 +203,6 @@ export function useChatRealtimeHandlers({
// hides it immediately and atomically. // hides it immediately and atomically.
onSessionIdle?.(sid); onSessionIdle?.(sid);
if (sid === activeViewSessionId) { if (sid === activeViewSessionId) {
pendingPermissionRequestsRef.current = [];
setPendingPermissionRequests([]); setPendingPermissionRequests([]);
} }
@@ -271,9 +235,9 @@ export function useChatRealtimeHandlers({
case 'permission_request': { case 'permission_request': {
if (!msg.requestId) break; if (!msg.requestId) break;
if (sid === activeViewSessionId) { if (sid === activeViewSessionId) {
const previousPendingPermissionRequests = pendingPermissionRequestsRef.current; setPendingPermissionRequests((prev) => {
if (!previousPendingPermissionRequests.some((request) => request.requestId === msg.requestId)) { if (prev.some((r: PendingPermissionRequest) => r.requestId === msg.requestId)) return prev;
const nextPendingPermissionRequests = [...previousPendingPermissionRequests, { return [...prev, {
requestId: msg.requestId as string, requestId: msg.requestId as string,
toolName: (msg.toolName as string) || 'UnknownTool', toolName: (msg.toolName as string) || 'UnknownTool',
input: msg.input, input: msg.input,
@@ -281,17 +245,7 @@ export function useChatRealtimeHandlers({
sessionId: sid || null, sessionId: sid || null,
receivedAt: new Date(), receivedAt: new Date(),
}]; }];
});
pendingPermissionRequestsRef.current = nextPendingPermissionRequests;
setPendingPermissionRequests(nextPendingPermissionRequests);
if (
isActionablePermissionRequest({ toolName: msg.toolName })
&& !hasActionablePermissionRequests(previousPendingPermissionRequests)
) {
void playNotificationSound();
}
}
} }
if (sid) { if (sid) {
onSessionProcessing?.(sid); onSessionProcessing?.(sid);
@@ -301,12 +255,7 @@ export function useChatRealtimeHandlers({
case 'permission_cancelled': { case 'permission_cancelled': {
if (msg.requestId && sid === activeViewSessionId) { if (msg.requestId && sid === activeViewSessionId) {
const nextPendingPermissionRequests = pendingPermissionRequestsRef.current.filter( setPendingPermissionRequests((prev) => prev.filter((r: PendingPermissionRequest) => r.requestId !== msg.requestId));
(request: PendingPermissionRequest) => request.requestId !== msg.requestId,
);
pendingPermissionRequestsRef.current = nextPendingPermissionRequests;
setPendingPermissionRequests(nextPendingPermissionRequests);
} }
break; break;
} }
@@ -337,7 +286,6 @@ export function useChatRealtimeHandlers({
selectedSession, selectedSession,
currentSessionId, currentSessionId,
setTokenBudget, setTokenBudget,
pendingPermissionRequests,
setPendingPermissionRequests, setPendingPermissionRequests,
streamTimerRef, streamTimerRef,
accumulatedStreamRef, accumulatedStreamRef,

View File

@@ -239,7 +239,6 @@ function ChatInterface({
selectedSession, selectedSession,
currentSessionId, currentSessionId,
setTokenBudget, setTokenBudget,
pendingPermissionRequests,
setPendingPermissionRequests, setPendingPermissionRequests,
streamTimerRef, streamTimerRef,
accumulatedStreamRef, accumulatedStreamRef,

View File

@@ -114,7 +114,7 @@
}, },
"sound": { "sound": {
"title": "Sound", "title": "Sound",
"description": "Play a short tone when a chat run finishes or needs tool approval.", "description": "Play a short tone when a chat run finishes.",
"enabled": "Enabled", "enabled": "Enabled",
"test": "Test sound" "test": "Test sound"
}, },

View File

@@ -58,7 +58,7 @@ const playTone = (
oscillator.stop(startsAt + duration + 0.02); oscillator.stop(startsAt + duration + 0.02);
}; };
export const playNotificationSound = async ({ force = false } = {}): Promise<void> => { export const playChatCompletionSound = async ({ force = false } = {}): Promise<void> => {
if (!force && !isNotificationSoundEnabled()) { if (!force && !isNotificationSoundEnabled()) {
return; return;
} }
@@ -81,5 +81,3 @@ export const playNotificationSound = async ({ force = false } = {}): Promise<voi
console.warn('Unable to play notification sound:', error); console.warn('Unable to play notification sound:', error);
} }
}; };
export const playChatCompletionSound = (options = {}): Promise<void> => playNotificationSound(options);