diff --git a/CHANGELOG.md b/CHANGELOG.md index c2260486..720e1db6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. - Work status: the session cost now counts what its subagents spent, with a line under the context meter splitting the session's own cost from the subagents' share, and each subagent's cost shown next to it in the Subagents list. Previously a session that delegated most of its work looked far cheaper than it was. - Files: opening a file over 5,000 lines is no longer blocked — the line-count guard now allows up to 20,000 lines, letting large files reach the virtualized full-file preview instead of being rejected at the open step (thanks @gaojunran). +- Usage: GitHub Copilot now shows a single AI Credits window, matching Copilot's token-based quota, in place of the old Chat Requests and Completions windows (thanks to @jakoss). - Settings: fixed the Cloudflare Tunnel download link shown when cloudflared is not installed (thanks to @AyoubAchour). ## [1.21.0] - 2026-08-26 diff --git a/packages/electron/main.mjs b/packages/electron/main.mjs index 49c2ba22..73b6e20a 100644 --- a/packages/electron/main.mjs +++ b/packages/electron/main.mjs @@ -33,7 +33,7 @@ import { } from './linux-autostart.mjs'; import { unsupportedAppSpecificOpenError, validateLocalPath } from './path-open-utils.mjs'; import { shouldAllowBrowserPanelCertificateError } from './browser-panel-security.mjs'; -import { createRendererRecoveryPolicy } from './renderer-recovery.mjs'; +import { attachRendererRecovery } from './renderer-recovery.mjs'; import { mintOutsideFileGrant } from '@openchamber/web/server/lib/fs/routes.js'; const execFileAsync = promisify(execFile); @@ -2497,7 +2497,6 @@ const createBrowserWindow = ({ label, restoreGeometry, url, runtimeConfig = {} } }; const browserWindow = new BrowserWindow(options); - const rendererRecoveryPolicy = createRendererRecoveryPolicy(); browserWindow.__ocLabel = label || nextWindowLabel(); browserWindow.__ocRuntimeConfig = { apiBaseUrl: desktopApiBaseUrl, clientToken: desktopClientToken, requestHeaders: desktopRequestHeaders }; browserWindow.__ocInitScript = buildInitScript(desktopLocalOrigin, state.bootOutcome, desktopApiBaseUrl, desktopClientToken, desktopRequestHeaders); @@ -2658,19 +2657,7 @@ const createBrowserWindow = ({ label, restoreGeometry, url, runtimeConfig = {} } browserWindow.webContents.on('zoom-changed', () => { browserWindow.webContents.setZoomFactor(1); }); - browserWindow.webContents.on('render-process-gone', (_event, details) => { - if (!rendererRecoveryPolicy.shouldReload(details.reason)) return; - log.warn('[electron] renderer exited unexpectedly; reloading window', { - label: browserWindow.__ocLabel, - reason: details.reason, - exitCode: details.exitCode, - }); - setTimeout(() => { - if (!browserWindow.isDestroyed()) { - browserWindow.webContents.reload(); - } - }, 100); - }); + attachRendererRecovery(browserWindow, { log, label: 'window' }); browserWindow.webContents.on('dom-ready', () => { if (browserWindow.__ocLabel === 'main') { @@ -2908,6 +2895,8 @@ const createMiniChatWindow = async ({ mode, sessionId = '', directory = '', proj browserWindow.__ocMiniChatSessionId = sessionWindowKey; browserWindow.__ocPinned = false; + attachRendererRecovery(browserWindow, { log, label: 'mini chat' }); + if (sessionWindowKey) { state.miniChatWindowsBySession.set(sessionWindowKey, browserWindow); } diff --git a/packages/electron/renderer-recovery.mjs b/packages/electron/renderer-recovery.mjs index 77037210..6e6b879a 100644 --- a/packages/electron/renderer-recovery.mjs +++ b/packages/electron/renderer-recovery.mjs @@ -8,6 +8,8 @@ const RECOVERABLE_REASONS = new Set([ 'memory-eviction', ]); +const RELOAD_DELAY_MS = 100; + export const createRendererRecoveryPolicy = (now = Date.now) => { let windowStartedAt = 0; let attempts = 0; @@ -28,3 +30,25 @@ export const createRendererRecoveryPolicy = (now = Date.now) => { }, }; }; + +/** + * Reload a window whose renderer process died, within the recovery budget. + * Shared by every BrowserWindow so the desktop shell has one recovery policy. + */ +export const attachRendererRecovery = (browserWindow, { log, label }) => { + const policy = createRendererRecoveryPolicy(); + browserWindow.webContents.on('render-process-gone', (_event, details) => { + if (!policy.shouldReload(details.reason)) return; + log.warn('[electron] renderer exited unexpectedly; reloading window', { + label: browserWindow.__ocLabel, + surface: label, + reason: details.reason, + exitCode: details.exitCode, + }); + setTimeout(() => { + if (!browserWindow.isDestroyed()) { + browserWindow.webContents.reload(); + } + }, RELOAD_DELAY_MS); + }); +}; diff --git a/packages/electron/renderer-recovery.test.mjs b/packages/electron/renderer-recovery.test.mjs index a0240051..76bf56a3 100644 --- a/packages/electron/renderer-recovery.test.mjs +++ b/packages/electron/renderer-recovery.test.mjs @@ -1,7 +1,34 @@ import assert from 'node:assert/strict'; import test from 'node:test'; +import { setTimeout } from 'node:timers/promises'; -import { createRendererRecoveryPolicy } from './renderer-recovery.mjs'; +import { attachRendererRecovery, createRendererRecoveryPolicy } from './renderer-recovery.mjs'; + +const createFakeWindow = () => { + const listeners = new Map(); + const state = { reloads: 0, destroyed: false }; + const browserWindow = { + __ocLabel: 'main', + state, + destroy: () => { + state.destroyed = true; + }, + emit: (event, details) => listeners.get(event)?.(null, details), + isDestroyed: () => state.destroyed, + webContents: { + on: (event, listener) => listeners.set(event, listener), + reload: () => { + state.reloads += 1; + }, + }, + }; + return browserWindow; +}; + +const createFakeLog = () => { + const warnings = []; + return { warnings, warn: (message, payload) => warnings.push({ message, payload }) }; +}; test('allows a bounded number of reloads for recoverable renderer failures', () => { const policy = createRendererRecoveryPolicy(() => 1_000); @@ -9,7 +36,20 @@ test('allows a bounded number of reloads for recoverable renderer failures', () assert.equal(policy.shouldReload('crashed'), true); assert.equal(policy.shouldReload('oom'), true); assert.equal(policy.shouldReload('abnormal-exit'), true); - assert.equal(policy.shouldReload('memory-eviction'), false); + assert.equal(policy.shouldReload('crashed'), false); +}); + +test('reloads after the renderer is evicted for memory', () => { + const policy = createRendererRecoveryPolicy(() => 1_000); + + assert.equal(policy.shouldReload('memory-eviction'), true); +}); + +test('ignores reasons Electron never reports for render-process-gone', () => { + const policy = createRendererRecoveryPolicy(() => 1_000); + + assert.equal(policy.shouldReload('made-up-reason'), false); + assert.equal(policy.shouldReload('crashed'), true); }); test('ignores clean and externally killed renderer exits', () => { @@ -32,3 +72,29 @@ test('resets the recovery budget after the recovery window', () => { currentTime += 60_000; assert.equal(policy.shouldReload('crashed'), true); }); + +test('reloads the attached window after a recoverable renderer failure', async () => { + const browserWindow = createFakeWindow(); + const log = createFakeLog(); + attachRendererRecovery(browserWindow, { log, label: 'mini chat' }); + + browserWindow.emit('render-process-gone', { reason: 'crashed', exitCode: 5 }); + await setTimeout(150); + + assert.equal(browserWindow.state.reloads, 1); + assert.equal(log.warnings.length, 1); + assert.equal(log.warnings[0].payload.surface, 'mini chat'); + assert.equal(log.warnings[0].payload.label, 'main'); +}); + +test('skips the reload when the window is gone or the exit is not recoverable', async () => { + const browserWindow = createFakeWindow(); + attachRendererRecovery(browserWindow, { log: createFakeLog(), label: 'window' }); + + browserWindow.emit('render-process-gone', { reason: 'clean-exit', exitCode: 0 }); + browserWindow.emit('render-process-gone', { reason: 'crashed', exitCode: 5 }); + browserWindow.destroy(); + await setTimeout(150); + + assert.equal(browserWindow.state.reloads, 0); +}); diff --git a/packages/ui/src/components/chat/ChatInput.tsx b/packages/ui/src/components/chat/ChatInput.tsx index 5362e86d..5b2880b0 100644 --- a/packages/ui/src/components/chat/ChatInput.tsx +++ b/packages/ui/src/components/chat/ChatInput.tsx @@ -36,7 +36,7 @@ import { ReviewFlowDialog, type ReviewFlowExecution } from '@/components/session import { BtwPanel } from './btw/BtwPanel'; import { useBtwPanelState } from './btw/useBtwPanelState'; import { wasPromotedBtwSession } from '@/lib/sessionBtwMetadata'; -import { BTW_BOUNDARY_INSTRUCTION, BTW_PROMOTION_NOTICE, destroyBtwSession, startBtwSession, type BtwSessionRef } from '@/lib/btw'; +import { buildBtwSyntheticTexts, destroyBtwSession, startBtwSession, type BtwSessionRef } from '@/lib/btw'; import { AttachedFilesList, AttachedVSCodeFileChips, ActiveEditorFileSuggestion } from './FileAttachment'; import { lazyWithChunkRecovery } from '@/lib/chunkLoadRecovery'; import type { ToolPopupContent } from './message/types'; @@ -1134,12 +1134,8 @@ const ChatInputComponent: React.FC = ({ composerText: !queuedOnly && inputSnapshot.hasContent ? inputSnapshot.message : null, composerAttachments: attachedFiles, inlineComments: drafts, - // btw mode: the boundary rides with every send, not just the - // first one, so the inherited transcript stays reference material - // for the whole side conversation. syntheticTexts: [ - ...(isBtwActive ? [BTW_BOUNDARY_INSTRUCTION] : []), - ...(isPromotedBtwSession ? [BTW_PROMOTION_NOTICE] : []), + ...buildBtwSyntheticTexts({ isBtwActive, isPromotedBtwSession }), ...(syntheticParts?.map((part) => part.text) ?? []), ], linkedIssue: linkedIssue diff --git a/packages/ui/src/components/chat/QuestionMarkdown.test.tsx b/packages/ui/src/components/chat/QuestionMarkdown.test.tsx index 2c3f1ef2..c13a9402 100644 --- a/packages/ui/src/components/chat/QuestionMarkdown.test.tsx +++ b/packages/ui/src/components/chat/QuestionMarkdown.test.tsx @@ -1,25 +1,35 @@ import { describe, expect, test } from 'bun:test'; +import { renderToStaticMarkup } from 'react-dom/server'; -import { SimpleMarkdownRenderer } from './MarkdownRenderer'; import { QuestionMarkdown } from './QuestionMarkdown'; +// The markdown renderer is lazy, so a synchronous server render always emits the +// Suspense fallback QuestionMarkdown supplies. That fallback is the surface that +// has to keep the exact question text and the question typography classes. describe('QuestionMarkdown', () => { - test('delegates exact content to the tool markdown renderer', () => { + test('renders the question content verbatim', () => { const content = 'Choose **one** from `mode`: [details](https://example.com)'; - const element = QuestionMarkdown({ content, size: 'meta' }); - expect(element.type).toBe(SimpleMarkdownRenderer); - expect(element.props.content).toBe(content); - expect(element.props.variant).toBe('tool'); - expect(element.props.fallbackContent.props.children).toBe(content); - expect(element.props.fallbackContent.props.className).toContain('whitespace-pre-wrap'); + const html = renderToStaticMarkup(); + + expect(html).toBe( + `
${content}
`, + ); }); - test('preserves question typography size and caller classes', () => { - const meta = QuestionMarkdown({ content: 'Meta', size: 'meta', className: 'font-medium text-foreground' }); - const micro = QuestionMarkdown({ content: 'Micro', size: 'micro', className: 'text-muted-foreground' }); + test('applies meta typography and caller classes', () => { + const html = renderToStaticMarkup( + , + ); - expect(meta.props.className).toBe('question-markdown typography-meta font-medium text-foreground'); - expect(micro.props.className).toBe('question-markdown typography-micro text-muted-foreground'); + expect(html).toContain('class="question-markdown typography-meta font-medium text-foreground whitespace-pre-wrap"'); + }); + + test('applies micro typography and caller classes', () => { + const html = renderToStaticMarkup( + , + ); + + expect(html).toContain('class="question-markdown typography-micro text-muted-foreground whitespace-pre-wrap"'); }); }); diff --git a/packages/ui/src/components/chat/composerHighlight.ts b/packages/ui/src/components/chat/composerHighlight.ts index 8252fc26..e0600a57 100644 --- a/packages/ui/src/components/chat/composerHighlight.ts +++ b/packages/ui/src/components/chat/composerHighlight.ts @@ -103,7 +103,7 @@ const STYLE_CLASS: Record = { mentionAgent: 'text-[var(--status-success)]', mentionCommand: 'text-[var(--primary)]', mentionSnippet: 'text-[var(--status-warning)]', - code: 'rounded-[6px] bg-[var(--markdown-inline-code-bg)] text-[var(--markdown-inline-code)] px-[0.3125rem] py-0.5', + code: 'rounded-[6px] bg-[var(--markdown-inline-code-bg)] text-[var(--markdown-inline-code)]', codeFence: 'bg-[var(--surface-subtle)] text-[var(--markdown-inline-code)]', // A `~path` is written for the reader's benefit, not to attach anything — // it takes the same colour as a file mention, since it names the same kind diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.tsx b/packages/ui/src/components/chat/message/parts/ToolPart.tsx index 78f6b41d..8ec48808 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.tsx +++ b/packages/ui/src/components/chat/message/parts/ToolPart.tsx @@ -46,9 +46,9 @@ import { buildTaskSummaryEntriesFromSession, normalizeTaskSummaryEntries, parseTaskMetadataBlock, + prepareTaskToolOutput, readTaskSessionIdFromOutput, readTaskSessionIdFromRecord, - stripTaskMetadataFromOutput, type TaskToolSummaryEntry, } from './taskToolModel'; import { areRenderRelevantPartsEqual } from '../renderCompare'; @@ -1004,9 +1004,7 @@ const TaskToolSummary: React.FC<{ const showToolFileIcons = useUIStore((state) => state.showToolFileIcons); const runtime = React.useContext(RuntimeAPIContext); - const trimmedOutput = typeof output === 'string' - ? stripTaskMetadataFromOutput(output) - : ''; + const trimmedOutput = prepareTaskToolOutput(output); const hasOutput = trimmedOutput.length > 0; const [isOutputExpanded, setIsOutputExpanded] = React.useState(false); @@ -2037,6 +2035,9 @@ const ToolPartContent: React.FC = ({ }; const handleMainKeyDown = (event: React.KeyboardEvent) => { + // Nested buttons (quick-open, copy) handle their own Enter/Space; the row + // must not swallow the key and toggle instead. + if (event.target !== event.currentTarget) return; if (event.key !== 'Enter' && event.key !== ' ') { return; } @@ -2092,13 +2093,6 @@ const ToolPartContent: React.FC = ({ openQuickTarget(); }; - const handleQuickOpenKeyDown = (event: React.KeyboardEvent) => { - if (event.key !== 'Enter' && event.key !== ' ') return; - event.preventDefault(); - event.stopPropagation(); - openQuickTarget(); - }; - const iconStyle = !isTaskTool && isError ? TOOL_ERROR_ICON_STYLE : TOOL_NORMAL_ICON_STYLE; const titleStyle = !isTaskTool && isError ? TOOL_ERROR_TITLE_STYLE : TOOL_NORMAL_TITLE_STYLE; const shouldRenderTaskSummary = useDeferredExpandedContent(isTaskTool && (taskSummaryEntries.length > 0 || isActive || shouldTreatAsFinalized || !!taskSessionId)); @@ -2192,7 +2186,7 @@ const ToolPartContent: React.FC = ({ {isExpanded ? : } -
+
= ({ , -})); - -mock.module('@/components/ui/input', () => ({ - Input: () => , -})); - -mock.module('@/components/ui', () => ({ - toast: { error: () => {}, success: () => {}, warning: () => {} }, -})); - -mock.module('@/lib/device', () => ({ - isMobileDeviceViaCSS: () => mobileDevice, -})); - -mock.module('@/components/ui/dialog', () => ({ - Dialog: ({ children }: ChildrenProps) => <>{children}, - DialogContent: ({ children }: ChildrenProps) =>
{children}
, - DialogDescription: ({ children }: ChildrenProps) =>
{children}
, - DialogFooter: ({ children }: ChildrenProps) =>
{children}
, - DialogHeader: ({ children }: ChildrenProps) =>
{children}
, - DialogTitle: ({ children }: ChildrenProps) =>
{children}
, -})); - -mock.module('@/components/ui/dropdown-menu', () => ({ - DropdownMenu: ({ children }: ChildrenProps) => <>{children}, - DropdownMenuContent: ({ children }: ChildrenProps) =>
{children}
, - DropdownMenuItem: ({ children, onClick }: ClickableProps) => { - if (React.Children.toArray(children).includes('Duplicate')) { - duplicateMenuClick = onClick ?? null; - } - return ; - }, - DropdownMenuTrigger: ({ children }: ChildrenProps) => <>{children}, -})); - -mock.module('@/components/ui/context-menu', () => ({ - ContextMenu: ({ children }: ChildrenProps) => <>{children}, - ContextMenuContent: ({ children }: ChildrenProps) =>
{children}
, - ContextMenuItem: ({ children }: ChildrenProps) =>
{children}
, - ContextMenuTrigger: ({ children, render }: TriggerProps) => <>{render}{children}, -})); - -mock.module('@/hooks/useSettingsDirectory', () => ({ - useSettingsDirectory: () => '/workspace', -})); - -mock.module('@/stores/useAgentsStore', () => ({ - useAgentsStore, - selectAgentsForDirectory: (state: AgentStoreState) => state.agents, - isAgentBuiltIn: () => false, - isAgentHidden: () => false, -})); - -mock.module('zustand/react/shallow', () => ({ useShallow })); - -mock.module('@/lib/utils', () => ({ - cn: (...classes: Array) => classes.filter(Boolean).join(' '), -})); - -mock.module('@/components/ui/ScrollableOverlay', () => ({ - ScrollableOverlay: ({ children }: ChildrenProps) =>
{children}
, -})); - -mock.module('@/components/sections/shared/SettingsProjectSelector', () => ({ - SettingsProjectSelector: () => null, -})); - -mock.module('@/components/sections/shared/SidebarGroup', () => ({ - SidebarGroup: ({ children }: ChildrenProps) => <>{children}, -})); - -mock.module('@/components/icon/Icon', () => ({ - Icon: () => null, -})); - -mock.module('@/lib/i18n', () => ({ - useI18n: () => ({ - t: (key: string) => (key === 'settings.common.actions.duplicate' ? 'Duplicate' : key), - }), -})); - -mock.module('@/components/sections/shared/SettingsSection', () => ({ - SETTINGS_PANEL_TITLE_CLASS: '', -})); - -const { AgentsSidebar } = await import('./AgentsSidebar'); - -function getDuplicateMenuClick(): ClickHandler { - if (!duplicateMenuClick) { - throw new Error('Expected the duplicate action to be rendered'); - } - return duplicateMenuClick; -} - -describe('AgentsSidebar duplicate action', () => { - test('notifies the mobile split-view parent once after preparing a prefilled agent draft', () => { - recordedDraft = null; - selectedAgentName = null; - duplicateMenuClick = null; - mobileDevice = true; - let mobileTransitionCount = 0; - - renderToStaticMarkup( - { mobileTransitionCount += 1; }} />, - ); - - getDuplicateMenuClick()({ stopPropagation: () => {} }); - - expect(recordedDraft).toEqual({ - name: 'writer-copy', - scope: 'project', - description: 'Writes concise documentation', - model: 'openai/gpt-4.1', - variant: 'fast', - temperature: 0.4, - top_p: 0.8, - prompt: 'Write clear documentation.', - mode: 'subagent', - permission: { bash: 'ask' }, - disable: true, - }); - expect(selectedAgentName).toBe('writer-copy'); - expect(mobileTransitionCount).toBe(1); - }); - - test('does not require a mobile transition callback on desktop', () => { - recordedDraft = null; - selectedAgentName = null; - duplicateMenuClick = null; - mobileDevice = false; - - renderToStaticMarkup(); - - getDuplicateMenuClick()({ stopPropagation: () => {} }); - expect(selectedAgentName).toBe('writer-copy'); - }); -}); diff --git a/packages/ui/src/components/ui/MemoryDebugPanel.tsx b/packages/ui/src/components/ui/MemoryDebugPanel.tsx index 957e654b..81227de3 100644 --- a/packages/ui/src/components/ui/MemoryDebugPanel.tsx +++ b/packages/ui/src/components/ui/MemoryDebugPanel.tsx @@ -172,7 +172,7 @@ const LineChart: React.FC<{ ); }; -export const DebugPanel: React.FC = ({ onClose }) => { +const DebugPanel: React.FC = ({ onClose }) => { const { t } = useI18n(); const [activeTab, setActiveTab] = React.useState('memory'); const [copyState, setCopyState] = React.useState<'idle' | 'copied' | 'error'>('idle'); diff --git a/packages/ui/src/components/views/SettingsView.mobile-focus.test.tsx b/packages/ui/src/components/views/SettingsView.mobile-focus.test.tsx deleted file mode 100644 index c69bbba1..00000000 --- a/packages/ui/src/components/views/SettingsView.mobile-focus.test.tsx +++ /dev/null @@ -1,438 +0,0 @@ -import React, { act } from 'react'; -import { describe, expect, mock, test } from 'bun:test'; -import { createRoot, type Root } from 'react-dom/client'; - -type ChildrenProps = { children?: React.ReactNode }; -type AgentsSidebarProps = { onItemSelect?: () => void }; -type SettingsPageLayoutProps = { - children: React.ReactNode; - title?: React.ReactNode; - showSaveStatus?: boolean; -}; - -interface FakeNode { - nodeType: number; - nodeName: string; - tagName: string; - namespaceURI: string; - ownerDocument: FakeDocument; - parentNode: FakeNode | null; - childNodes: FakeNode[]; - style: { setProperty: () => void; getPropertyValue: () => string }; - classList: FakeClassList; - attributes: Map; - textContent: string; - nodeValue: string | null; - focusOptions?: FocusOptions; - appendChild: (child: FakeNode) => FakeNode; - insertBefore: (child: FakeNode, before: FakeNode | null) => FakeNode; - removeChild: (child: FakeNode) => FakeNode; - setAttribute: (name: string, value: string) => void; - removeAttribute: (name: string) => void; - getAttribute: (name: string) => string | null; - hasAttribute: (name: string) => boolean; - addEventListener: () => void; - removeEventListener: () => void; - contains: (child: FakeNode | null) => boolean; - querySelector: (selector: string) => FakeNode | null; - focus: (options?: FocusOptions) => void; -} - -interface FakeDocument { - nodeType: number; - nodeName: string; - defaultView: FakeWindow | null; - body: FakeNode | null; - documentElement: FakeNode | null; - activeElement: FakeNode | null; - createElement: (tag: string) => FakeNode & Element; - createElementNS: (_namespace: string, tag: string) => FakeNode & Element; - createTextNode: (text: string) => FakeNode & Element; - addEventListener: () => void; - removeEventListener: () => void; -} - -interface FakeWindow { - document: FakeDocument; - navigator: { userAgent: string; platform: string; maxTouchPoints: number }; - history: { state: null; back: () => void; pushState: () => void }; - location: { href: string }; - requestAnimationFrame: (callback: FrameRequestCallback) => number; - cancelAnimationFrame: (frame: number) => void; - addEventListener: () => void; - removeEventListener: () => void; - HTMLIFrameElement: typeof FakeElement; - HTMLFrameSetElement: typeof FakeElement; - HTMLInputElement: typeof FakeElement; - HTMLTextAreaElement: typeof FakeElement; - HTMLSelectElement: typeof FakeElement; - HTMLOptionElement: typeof FakeElement; - HTMLAnchorElement: typeof FakeElement; -} - -type GlobalStubValue = FakeDocument | FakeWindow | FakeWindow['navigator'] | FakeWindow['location'] | typeof FakeElement | boolean; - -class FakeElement {} - -class FakeClassList { - private readonly classes = new Set(); - - add(...classes: string[]) { - classes.forEach((className) => this.classes.add(className)); - } - - remove(...classes: string[]) { - classes.forEach((className) => this.classes.delete(className)); - } - - contains(className: string) { - return this.classes.has(className); - } -} - -function makeNode(tag: string, ownerDocument: FakeDocument, nodeType = 1): FakeNode & Element { - const attributes = new Map(); - const properties: FakeNode = { - nodeType, - nodeName: nodeType === 3 ? '#text' : tag.toUpperCase(), - tagName: nodeType === 3 ? '#text' : tag.toUpperCase(), - namespaceURI: 'http://www.w3.org/1999/xhtml', - ownerDocument, - parentNode: null, - childNodes: [], - style: { - setProperty: () => {}, - getPropertyValue: () => '', - }, - classList: new FakeClassList(), - attributes, - textContent: '', - nodeValue: null, - appendChild(child) { - this.childNodes.push(child); - child.parentNode = this; - return child; - }, - insertBefore(child, before) { - const index = before ? this.childNodes.indexOf(before) : -1; - if (index === -1) { - this.childNodes.push(child); - } else { - this.childNodes.splice(index, 0, child); - } - child.parentNode = this; - return child; - }, - removeChild(child) { - const index = this.childNodes.indexOf(child); - if (index !== -1) { - this.childNodes.splice(index, 1); - } - child.parentNode = null; - return child; - }, - setAttribute(name, value) { - attributes.set(name, value); - }, - removeAttribute(name) { - attributes.delete(name); - }, - getAttribute(name) { - return attributes.get(name) ?? null; - }, - hasAttribute(name) { - return attributes.has(name); - }, - addEventListener: () => {}, - removeEventListener: () => {}, - contains(child) { - if (child === this) { - return true; - } - return this.childNodes.some((nodeChild) => nodeChild.contains(child)); - }, - querySelector(selector) { - if (selector !== '[data-settings-page-heading]') { - return null; - } - if (this.hasAttribute('data-settings-page-heading')) { - return this; - } - for (const child of this.childNodes) { - const match = child.querySelector(selector); - if (match) { - return match; - } - } - return null; - }, - focus(options) { - this.focusOptions = options; - this.ownerDocument.activeElement = this; - }, - }; - const node: FakeNode & Element = Object.assign(Object.create(FakeElement.prototype), properties); - return node; -} - -function installDomStub() { - const descriptors = new Map(); - const setGlobal = (name: string, value: GlobalStubValue) => { - descriptors.set(name, Object.getOwnPropertyDescriptor(globalThis, name)); - Object.defineProperty(globalThis, name, { configurable: true, writable: true, value }); - }; - const frames = new Map(); - let nextFrame = 1; - const documentStub: FakeDocument = { - nodeType: 9, - nodeName: '#document', - defaultView: null, - body: null, - documentElement: null, - activeElement: null, - createElement: (tag) => makeNode(tag, documentStub), - createElementNS: (_namespace, tag) => makeNode(tag, documentStub), - createTextNode: (text) => { - const node = makeNode('#text', documentStub, 3); - node.nodeValue = text; - node.textContent = text; - return node; - }, - addEventListener: () => {}, - removeEventListener: () => {}, - }; - const windowStub: FakeWindow = { - document: documentStub, - navigator: { userAgent: 'test', platform: 'test', maxTouchPoints: 0 }, - history: { state: null, back: () => {}, pushState: () => {} }, - location: { href: 'http://localhost/' }, - requestAnimationFrame: (callback) => { - const frame = nextFrame; - nextFrame += 1; - frames.set(frame, callback); - return frame; - }, - cancelAnimationFrame: (frame) => { - frames.delete(frame); - }, - addEventListener: () => {}, - removeEventListener: () => {}, - HTMLIFrameElement: FakeElement, - HTMLFrameSetElement: FakeElement, - HTMLInputElement: FakeElement, - HTMLTextAreaElement: FakeElement, - HTMLSelectElement: FakeElement, - HTMLOptionElement: FakeElement, - HTMLAnchorElement: FakeElement, - }; - documentStub.defaultView = windowStub; - documentStub.body = makeNode('body', documentStub); - documentStub.documentElement = makeNode('html', documentStub); - documentStub.activeElement = documentStub.body; - - setGlobal('document', documentStub); - setGlobal('window', windowStub); - setGlobal('navigator', windowStub.navigator); - setGlobal('location', windowStub.location); - setGlobal('Element', FakeElement); - setGlobal('HTMLElement', FakeElement); - setGlobal('HTMLIFrameElement', FakeElement); - setGlobal('IS_REACT_ACT_ENVIRONMENT', true); - - return { - container: documentStub.createElement('div'), - document: documentStub, - frameCount: () => frames.size, - flushFrames: () => { - const callbacks = Array.from(frames.values()); - frames.clear(); - callbacks.forEach((callback) => callback(Date.now())); - }, - restore: () => { - for (const [name, descriptor] of descriptors) { - if (descriptor) { - Object.defineProperty(globalThis, name, descriptor); - } else { - Reflect.deleteProperty(globalThis, name); - } - } - }, - }; -} - -const Empty = () => null; -const uiStore = { - settingsPage: 'agents', - isSettingsDialogOpen: true, - setSettingsPage: () => {}, -}; -type UiStoreValue = (typeof uiStore)[keyof typeof uiStore]; -const agentsMeta = { slug: 'agents', title: 'Agents', group: 'opencode', kind: 'split' }; -let sidebarOnItemSelect: (() => void) | undefined; -let SettingsPageLayout: React.ComponentType | null = null; - -mock.module('@/lib/utils', () => ({ - cn: (...classes: Array) => classes.filter(Boolean).join(' '), - getModifierLabel: () => 'Ctrl', -})); -mock.module('@/stores/useUIStore', () => ({ - useUIStore: (selector: (state: typeof uiStore) => UiStoreValue) => selector(uiStore), -})); -mock.module('@/hooks/useSettingsDirectory', () => ({ useSettingsDirectory: () => '/workspace' })); -mock.module('@/stores/useProjectsStore', () => ({ - useProjectsStore: (selector: (state: { activeProjectId: null }) => null) => selector({ activeProjectId: null }), -})); -mock.module('@/stores/useAgentsStore', () => ({ - refreshAfterOpenCodeRestart: async () => {}, - useAgentsStore: { getState: () => ({ loadAgents: async () => {} }) }, -})); -mock.module('@/stores/useCommandsStore', () => ({ useCommandsStore: { getState: () => ({ loadCommands: async () => {} }) } })); -mock.module('@/stores/useMcpConfigStore', () => ({ useMcpConfigStore: { getState: () => ({ loadMcpConfigs: async () => {} }) } })); -mock.module('@/stores/useSnippetsStore', () => ({ useSnippetsStore: { getState: () => ({ loadSnippets: async () => {} }) } })); -mock.module('@/stores/useSkillsStore', () => ({ useSkillsStore: { getState: () => ({ loadSkills: async () => {} }) } })); -mock.module('@/stores/useSkillsCatalogStore', () => ({ useSkillsCatalogStore: { getState: () => ({ loadCatalog: async () => {} }) } })); -mock.module('@/stores/useConfigStore', () => ({ useConfigStore: { getState: () => ({ providers: [], setSelectedProvider: () => {} }) } })); -mock.module('@/stores/usePendingOpenCodeRestartStore', () => ({ - selectPendingOpenCodeRestartCount: () => 0, - usePendingOpenCodeRestartStore: () => 0, -})); -mock.module('@/components/ui/tooltip', () => ({ - Tooltip: ({ children }: ChildrenProps) => <>{children}, - TooltipTrigger: ({ children }: ChildrenProps) => <>{children}, -})); -mock.module('@/components/ui/ErrorBoundary', () => ({ ErrorBoundary: ({ children }: ChildrenProps) => <>{children} })); -mock.module('@/components/ui/ScrollableOverlay', () => ({ ScrollableOverlay: ({ children }: ChildrenProps) =>
{children}
})); -mock.module('@/components/sections/shared/SettingsSection', () => ({ - SETTINGS_DESCRIPTION_CLASS: '', - SETTINGS_PAGE_TITLE_CLASS: '', - SETTINGS_SECTION_TITLE_CLASS: '', -})); -mock.module('@/lib/persistence', () => ({ - getSettingsSaveState: () => 'idle', - subscribeToSettingsSaveState: () => () => {}, -})); -mock.module('@/components/icon/Icon', () => ({ Icon: Empty })); -mock.module('@/components/icons/McpIcon', () => ({ McpIcon: Empty })); -mock.module('@/lib/i18n', () => ({ useI18n: () => ({ t: (key: string) => key }) })); -mock.module('@/lib/device', () => ({ - useDeviceInfo: () => ({ isMobile: false }), -})); -mock.module('@/lib/desktop', () => ({ - getDesktopHomeDirectory: async () => null, - isDesktopLocalOriginActive: () => false, - isDesktopShell: () => false, - isVSCodeRuntime: () => false, - isWebRuntime: () => true, -})); -mock.module('@/lib/platform', () => ({ isWindowsArm64: () => false })); -mock.module('@/lib/settings/metadata', () => ({ - SETTINGS_PAGE_METADATA: [agentsMeta], - getSettingsNavIcon: () => 'settings-3', - getSettingsPageMeta: (slug: string) => slug === 'agents' ? agentsMeta : null, - resolveSettingsSlug: (slug: string) => slug === 'agents' ? 'agents' : 'home', -})); -mock.module('@/lib/settings/search', () => ({ buildSettingsSearchResults: () => [] })); -mock.module('@/components/views/OpenCodeReloadFooterAction', () => ({ OpenCodeReloadFooterAction: Empty })); -mock.module('@/components/sections/agents/AgentsSidebar', () => ({ - AgentsSidebar: ({ onItemSelect }: AgentsSidebarProps) => { - sidebarOnItemSelect = onItemSelect; - return ; - }, -})); -mock.module('@/components/sections/agents/AgentsPage', () => ({ - AgentsPage: () => { - const Layout = SettingsPageLayout; - if (!Layout) { - throw new Error('SettingsPageLayout must load before SettingsView'); - } - return
; - }, -})); - -for (const [module, exports] of [ - ['@/components/sections/behavior/BehaviorPage', ['BehaviorPage']], - ['@/components/sections/commands/CommandsSidebar', ['CommandsSidebar']], - ['@/components/sections/commands/CommandsPage', ['CommandsPage']], - ['@/components/sections/mcp/McpSidebar', ['McpSidebar']], - ['@/components/sections/mcp/McpPage', ['McpPage']], - ['@/components/sections/plugins', ['PluginsSidebar', 'PluginsPage']], - ['@/components/sections/skills/SkillsSidebar', ['SkillsSidebar']], - ['@/components/sections/skills/SkillsPage', ['SkillsPage']], - ['@/components/sections/projects/ProjectsSidebar', ['ProjectsSidebar']], - ['@/components/sections/projects/ProjectsPage', ['ProjectsPage']], - ['@/components/sections/remote-instances/RemoteInstancesPage', ['RemoteInstancesPage']], - ['@/components/sections/providers/ProvidersSidebar', ['ProvidersSidebar']], - ['@/components/sections/providers/ProvidersPage', ['ProvidersPage']], - ['@/components/sections/usage/UsageSidebar', ['UsageSidebar']], - ['@/components/sections/usage/UsagePage', ['UsagePage']], - ['@/components/sections/magic-prompts/MagicPromptsSidebar', ['MagicPromptsSidebar']], - ['@/components/sections/magic-prompts/MagicPromptsPage', ['MagicPromptsPage']], - ['@/components/sections/snippets/SnippetsSidebar', ['SnippetsSidebar']], - ['@/components/sections/snippets/SnippetsPage', ['SnippetsPage']], - ['@/components/sections/git-identities/GitPage', ['GitPage']], - ['@/components/sections/integrations/IntegrationsPage', ['IntegrationsPage']], - ['@/components/sections/openchamber/OpenChamberPage', ['OpenChamberPage']], - ['@/components/sections/openchamber/AboutSettings', ['AboutSettings']], -] as const) { - mock.module(module, () => Object.fromEntries(exports.map((name) => [name, Empty]))); -} - -SettingsPageLayout = (await import('../sections/shared/SettingsPageLayout')).SettingsPageLayout; -const { SettingsView } = await import('./SettingsView'); - -describe('SettingsView mobile split-page focus', () => { - test('focuses the rendered editor heading after a mobile sidebar selection', async () => { - const dom = installDomStub(); - const root: Root = createRoot(dom.container); - sidebarOnItemSelect = undefined; - - try { - await act(async () => { - root.render(); - }); - - expect(sidebarOnItemSelect).toBeDefined(); - await act(async () => { - sidebarOnItemSelect?.(); - }); - - const heading = dom.container.querySelector('[data-settings-page-heading]'); - expect(heading).not.toBeNull(); - expect(heading?.getAttribute('tabindex')).toBe('-1'); - expect(dom.document.activeElement).toBe(dom.document.body); - expect(dom.frameCount()).toBe(1); - - await act(async () => { - dom.flushFrames(); - }); - - expect(dom.document.activeElement).toBe(heading); - expect(heading?.focusOptions).toEqual({ preventScroll: true }); - } finally { - await act(async () => { - root.unmount(); - }); - dom.restore(); - } - }); - - test('does not pass the mobile selection callback to desktop split pages', async () => { - const dom = installDomStub(); - const root: Root = createRoot(dom.container); - sidebarOnItemSelect = undefined; - - try { - await act(async () => { - root.render(); - }); - - expect(sidebarOnItemSelect).toBe(undefined); - expect(dom.frameCount()).toBe(0); - } finally { - await act(async () => { - root.unmount(); - }); - dom.restore(); - } - }); -}); diff --git a/packages/ui/src/lib/btw.test.ts b/packages/ui/src/lib/btw.test.ts index 80e004b1..4b13285e 100644 --- a/packages/ui/src/lib/btw.test.ts +++ b/packages/ui/src/lib/btw.test.ts @@ -58,7 +58,7 @@ mock.module('@/sync/sync-refs', () => ({ }), })); -const { btwSessionTitle, startBtwSession, destroyBtwSession, promoteBtwSession, filterBtwTailMessages, findLastCompletedAssistantMessageID, BTW_BOUNDARY_INSTRUCTION } = +const { btwSessionTitle, startBtwSession, destroyBtwSession, promoteBtwSession, filterBtwTailMessages, findLastCompletedAssistantMessageID, BTW_BOUNDARY_INSTRUCTION, BTW_PROMOTION_NOTICE, buildBtwSyntheticTexts } = await import('@/lib/btw'); const { useBtwStore } = await import('@/stores/useBtwStore'); @@ -304,3 +304,24 @@ describe('promoteBtwSession', () => { expect(currentSessionSwitches).toEqual([]); }); }); + +describe('buildBtwSyntheticTexts', () => { + test('a send routed to an active fork carries only the boundary instruction', () => { + // Regression: a promoted parent that opens a new btw fork used to send the + // promotion notice into the fork alongside the boundary instruction, telling + // the fork both that btw constraints apply and that they no longer apply. + expect(buildBtwSyntheticTexts({ isBtwActive: true, isPromotedBtwSession: true })) + .toEqual([BTW_BOUNDARY_INSTRUCTION]); + expect(buildBtwSyntheticTexts({ isBtwActive: true, isPromotedBtwSession: false })) + .toEqual([BTW_BOUNDARY_INSTRUCTION]); + }); + + test('a promoted session with no active fork carries the promotion notice', () => { + expect(buildBtwSyntheticTexts({ isBtwActive: false, isPromotedBtwSession: true })) + .toEqual([BTW_PROMOTION_NOTICE]); + }); + + test('an ordinary session carries neither', () => { + expect(buildBtwSyntheticTexts({ isBtwActive: false, isPromotedBtwSession: false })).toEqual([]); + }); +}); diff --git a/packages/ui/src/lib/btw.ts b/packages/ui/src/lib/btw.ts index 32fa52f8..dd5e1233 100644 --- a/packages/ui/src/lib/btw.ts +++ b/packages/ui/src/lib/btw.ts @@ -74,6 +74,23 @@ export const BTW_PROMOTION_NOTICE = + 'The btw constraints in the history above no longer apply: this is now the main thread, and the ' + 'usual tool, sub-agent and workspace permissions are in force.'; +/** + * The btw framing texts a composer send carries. + * + * The boundary instruction rides with every send routed to an active btw fork, + * so the inherited transcript stays reference material for the whole side + * conversation. The promotion notice is the opposite case: it tells a promoted + * session that the btw constraints in its own history are lifted. A send routed + * to a fresh fork is never that session, so the two never travel together. + */ +export const buildBtwSyntheticTexts = (state: { + isBtwActive: boolean; + isPromotedBtwSession: boolean; +}): string[] => { + if (state.isBtwActive) return [BTW_BOUNDARY_INSTRUCTION]; + return state.isPromotedBtwSession ? [BTW_PROMOTION_NOTICE] : []; +}; + /** The boundary as an `additionalParts` entry for `sendMessage`. */ const btwBoundaryParts = (): Array<{ text: string; synthetic: true }> => [{ text: BTW_BOUNDARY_INSTRUCTION, synthetic: true }]; diff --git a/packages/ui/src/lib/i18n/messages/de.ts b/packages/ui/src/lib/i18n/messages/de.ts index 81f66485..62c0cfaa 100644 --- a/packages/ui/src/lib/i18n/messages/de.ts +++ b/packages/ui/src/lib/i18n/messages/de.ts @@ -1779,8 +1779,6 @@ export const dict = { 'session.newWorktree.noMatchingBranches': 'Keine übereinstimmenden Branches', 'session.newWorktree.localBranches': 'Lokale Branches', 'session.newWorktree.remoteBranches': 'Remote-Branches', - 'session.newWorktree.otherLocalBranches': 'Andere lokale Branches', - 'session.newWorktree.otherRemoteBranches': 'Andere Remote-Branches', 'session.newWorktree.branchName': 'Branch-Name', 'session.newWorktree.branchNamePlaceholder': 'feature/mein-geil-feature', 'session.newWorktree.actions.change': 'Ändern', @@ -2811,6 +2809,7 @@ export const dict = { 'memoryDebugPanel.title': 'Debug Panel', 'memoryDebugPanel.tabs.memory': 'Speicher', 'memoryDebugPanel.tabs.streaming': 'Streaming', + 'memoryDebugPanel.tabs.requests': 'Anfragen', 'memoryDebugPanel.section.sessionsInMemory': 'Sitzungen im Speicher', 'memoryDebugPanel.section.uiStreamingMetrics': 'UI-Streaming-Metriken', 'memoryDebugPanel.section.vscodeBridgeMetrics': 'VS Code Bridge Metriken', @@ -2848,6 +2847,16 @@ export const dict = { 'memoryDebugPanel.streaming.copy.copied': 'Streaming-Debug-JSON kopiert', 'memoryDebugPanel.streaming.copy.failed': 'Fehler beim Kopieren der JSON-Datei', 'memoryDebugPanel.streaming.copy.hint': 'Kopieren exportiert sowohl UI- als auch VS Code-Streaming-Metriken als JSON', + 'memoryDebugPanel.requests.inFlight': 'Laufend', + 'memoryDebugPanel.requests.peak': 'Spitze', + 'memoryDebugPanel.requests.duration': 'Dauer', + 'memoryDebugPanel.requests.totalRequests': 'Gesamtanfragen', + 'memoryDebugPanel.requests.tracking': 'Aufzeichnung', + 'memoryDebugPanel.requests.now': 'jetzt', + 'memoryDebugPanel.requests.noSamples': 'Noch keine Anfragen aufgezeichnet. Lassen Sie dieses Panel geöffnet, um Fetch-Aktivität zu erfassen.', + 'memoryDebugPanel.requests.chartLabel': 'Laufende Fetch-Anfragen im Zeitverlauf, Spitze {peak}', + 'memoryDebugPanel.requests.windowHint': 'letzte {seconds}s', + 'memoryDebugPanel.requests.percentileChartLabel': 'Perzentile des Alters laufender Anfragen (p50, p90, p99, max) im Zeitverlauf', 'memoryDebugPanel.common.idle': 'inaktiv', 'memoryDebugPanel.common.live': 'live', 'memoryDebugPanel.common.notAvailable': 'n/a', diff --git a/packages/ui/src/lib/i18n/messages/ja.ts b/packages/ui/src/lib/i18n/messages/ja.ts index fa54004a..845bc896 100644 --- a/packages/ui/src/lib/i18n/messages/ja.ts +++ b/packages/ui/src/lib/i18n/messages/ja.ts @@ -1954,8 +1954,6 @@ export const dict: Record = { 'session.newWorktree.noMatchingBranches': '一致するブランチがありません', 'session.newWorktree.localBranches': 'ローカルブランチ', 'session.newWorktree.remoteBranches': 'リモートブランチ', - 'session.newWorktree.otherLocalBranches': 'その他のローカルブランチ', - 'session.newWorktree.otherRemoteBranches': 'その他のリモートブランチ', 'session.newWorktree.branchName': 'ブランチ名', 'session.newWorktree.branchNamePlaceholder': 'feature/my-awesome-feature', 'session.newWorktree.actions.change': '変更', diff --git a/packages/ui/src/lib/i18n/messages/uk.ts b/packages/ui/src/lib/i18n/messages/uk.ts index dfe28604..08a5949c 100644 --- a/packages/ui/src/lib/i18n/messages/uk.ts +++ b/packages/ui/src/lib/i18n/messages/uk.ts @@ -3022,7 +3022,7 @@ export const dict: Record = { "memoryDebugPanel.requests.now": "зараз", "memoryDebugPanel.requests.noSamples": "Запитів ще немає. Тримайте цю панель відкритою, щоб фіксувати активність fetch.", "memoryDebugPanel.requests.chartLabel": "Запити fetch у виконанні з часом, пік {peak}", - "memoryDebugPanel.requests.windowHint": "останні {seconds}s", + "memoryDebugPanel.requests.windowHint": "останні {seconds} с", "memoryDebugPanel.requests.percentileChartLabel": "Перцентилі віку запитів у виконанні (p50, p90, p99, max) з часом", "memoryDebugPanel.common.idle": "очікування", "memoryDebugPanel.common.live": "live", diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index c5fbb833..88558e7d 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -268,7 +268,7 @@ Rules: 6. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session. 7. Regular new-chat drafts that inherit the persisted current/last directory must not create a session against a confirmed-missing path. Fall back to the active project only when OpenCode reports the directory missing; keep explicit worktree targets, in-flight worktree creation, and unknown/offline probes unchanged, and do not persist the fallback until session creation succeeds. A concurrent draft rewrite to that same active-project fallback must not abort session creation. 8. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message. -9. Revert and unrevert cascade through known descendant sessions before mutating the parent. Revert uses the first descendant user message at or after the parent's target timestamp, including equal timestamps because message IDs do not define chronology. A descendant failure is logged and does not block its siblings or the parent. The parent runs last so its shared-directory file snapshot remains authoritative. +9. Revert and unrevert cascade through known descendant sessions before mutating the parent. Revert uses the first descendant user message at or after the parent's target timestamp, including equal timestamps because message IDs do not define chronology. A descendant failure is logged and does not block its siblings or the parent. The parent runs last so its shared-directory file snapshot remains authoritative. A busy descendant is aborted before it is reverted, like the parent, so nothing keeps writing past the revert boundary. Redo clears the revert marker on every descendant, including markers the user set on a subagent independently of the parent undo. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/session-actions.test.ts b/packages/ui/src/sync/session-actions.test.ts index b0fee010..d4611074 100644 --- a/packages/ui/src/sync/session-actions.test.ts +++ b/packages/ui/src/sync/session-actions.test.ts @@ -16,6 +16,7 @@ let sessionMessagesResult: { data?: unknown; error?: unknown; response?: { statu const sessionMessageRecords = new Map>() const failingRevertSessionIds = new Set() const failingUnrevertSessionIds = new Set() +let afterUnrevertCall: ((sessionId: string) => void) | null = null let sessionDeleteError: unknown | null = null let beforeSessionUpdateResolve: ((sessionId: string) => void) | null = null let beforeSessionDeleteResolve: ((sessionId: string) => void) | null = null @@ -73,6 +74,7 @@ const mockSdk = { }), unrevert: mock((params: Record) => { replyCalls.push({ method: "session.unrevert", params }) + afterUnrevertCall?.(String(params.sessionID)) if (failingUnrevertSessionIds.has(String(params.sessionID))) { return Promise.resolve({ error: { message: "rejected" }, response: { status: 500 } }) } @@ -1435,6 +1437,44 @@ describe("revertToMessage passes session directory", () => { "root", ]) }) + + test("aborts a busy descendant before reverting it", async () => { + const rootMessage = { id: "root-cutoff", sessionID: "root", role: "user", time: { created: 20 } } as Message + const sessions = [ + { id: "root", directory: "/tree", time: { created: 1 } }, + { id: "busy-child", parentID: "root", directory: "/tree", time: { created: 2 } }, + { id: "idle-child", parentID: "root", directory: "/tree", time: { created: 3 } }, + ] as Session[] + const store = createStore({}, { + session: sessions, + message: { root: [rootMessage] }, + session_status: { "busy-child": { type: "busy" }, "idle-child": { type: "idle" } }, + }) + for (const id of ["busy-child", "idle-child"]) { + sessionMessageRecords.set(id, [{ + info: { id: `${id}-target`, sessionID: id, role: "user", time: { created: 20 } } as Message, + parts: [], + }]) + } + + const { setActionRefs, revertToMessage } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, createChildStores([["/tree", store]]), () => "/tree") + + await revertToMessage("root", "root-cutoff") + + expect(replyCalls.filter((call) => call.method === "session.abort").map((call) => call.params.sessionID)) + .toEqual(["busy-child"]) + const busyAbortIndex = replyCalls.findIndex((call) => call.method === "session.abort") + const busyRevertIndex = replyCalls.findIndex( + (call) => call.method === "session.revert" && call.params.sessionID === "busy-child", + ) + expect(busyAbortIndex).toBeLessThan(busyRevertIndex) + expect(replyCalls.filter((call) => call.method === "session.revert").map((call) => call.params.sessionID)).toEqual([ + "busy-child", + "idle-child", + "root", + ]) + }) }) describe("unrevertSession descendant cascade", () => { @@ -1442,6 +1482,7 @@ describe("unrevertSession descendant cascade", () => { replyCalls.length = 0 sessionMessagesResult = { data: [] } failingUnrevertSessionIds.clear() + afterUnrevertCall = null }) test("unreverts only marked descendants before the parent", async () => { @@ -1485,6 +1526,75 @@ describe("unrevertSession descendant cascade", () => { "root", ]) }) + + test("aborts a busy descendant before unreverting it", async () => { + const sessions = [ + { id: "root", directory: "/tree", time: { created: 1 }, revert: { messageID: "root-target" } }, + { id: "busy-child", parentID: "root", directory: "/tree", time: { created: 2 }, revert: { messageID: "busy-target" } }, + { id: "idle-child", parentID: "root", directory: "/tree", time: { created: 3 }, revert: { messageID: "idle-target" } }, + ] as Session[] + const store = createStore({}, { + session: sessions, + session_status: { "busy-child": { type: "busy" }, "idle-child": { type: "idle" } }, + }) + + const { setActionRefs, unrevertSession } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, createChildStores([["/tree", store]]), () => "/tree") + + await unrevertSession("root") + + expect(replyCalls.filter((call) => call.method === "session.abort").map((call) => call.params.sessionID)) + .toEqual(["busy-child"]) + const abortIndex = replyCalls.findIndex((call) => call.method === "session.abort") + const unrevertIndex = replyCalls.findIndex( + (call) => call.method === "session.unrevert" && call.params.sessionID === "busy-child", + ) + expect(abortIndex).toBeLessThan(unrevertIndex) + }) + + test("treats a descendant as busy when any child store reports a non-idle status", async () => { + const sessions = [ + { id: "root", directory: "/tree", time: { created: 1 }, revert: { messageID: "root-target" } }, + { id: "busy-child", parentID: "root", directory: "/tree", time: { created: 2 }, revert: { messageID: "busy-target" } }, + ] as Session[] + // The session list is deduped onto /tree, but the live status arrived in the + // store for another directory. + const treeStore = createStore({}, { session: sessions }) + const statusStore = createStore({}, { session_status: { "busy-child": { type: "busy" } } }) + + const { setActionRefs, unrevertSession } = await import("./session-actions") + setActionRefs( + mockSdk as unknown as OpencodeClient, + createChildStores([["/tree", treeStore], ["/other", statusStore]]), + () => "/tree", + ) + + await unrevertSession("root") + + expect(replyCalls.filter((call) => call.method === "session.abort").map((call) => call.params.sessionID)) + .toEqual(["busy-child"]) + }) + + test("aborts a descendant that turns busy after the subtree snapshot", async () => { + const sessions = [ + { id: "root", directory: "/tree", time: { created: 1 }, revert: { messageID: "root-target" } }, + { id: "first-child", parentID: "root", directory: "/tree", time: { created: 2 }, revert: { messageID: "first-target" } }, + { id: "second-child", parentID: "root", directory: "/tree", time: { created: 3 }, revert: { messageID: "second-target" } }, + ] as Session[] + const store = createStore({}, { session: sessions, session_status: {} }) + afterUnrevertCall = (sessionId) => { + if (sessionId !== "first-child") return + store.getState().patch({ session_status: { "second-child": { type: "busy" } } }) + } + + const { setActionRefs, unrevertSession } = await import("./session-actions") + setActionRefs(mockSdk as unknown as OpencodeClient, createChildStores([["/tree", store]]), () => "/tree") + + await unrevertSession("root") + + expect(replyCalls.filter((call) => call.method === "session.abort").map((call) => call.params.sessionID)) + .toEqual(["second-child"]) + }) }) describe("dismissPermission passes directory", () => { diff --git a/packages/ui/src/sync/session-actions.ts b/packages/ui/src/sync/session-actions.ts index ea4a7dd8..03b277e0 100644 --- a/packages/ui/src/sync/session-actions.ts +++ b/packages/ui/src/sync/session-actions.ts @@ -443,13 +443,40 @@ type DescendantSession = { directory: string } +/** + * A session's live status can live in a different child store than the one that + * wins the directory dedup, so any store reporting a non-idle status counts. + * Read at the moment of use: a descendant can start working after the subtree + * snapshot was taken. + */ +function isSessionBusyNow(sessionId: string): boolean { + const stores = _childStores + if (!stores) return false + + for (const [, store] of stores.children) { + const status = store.getState().session_status?.[sessionId] + if (status && status.type !== "idle") return true + } + return false +} + +async function abortDescendantIfBusy(sessionId: string, directory: string): Promise { + if (!isSessionBusyNow(sessionId)) return + try { + await sdk().session.abort({ sessionID: sessionId, directory }) + } catch { + // ignore abort errors + } +} + function getDescendantSessions(rootId: string): DescendantSession[] { const stores = _childStores if (!stores) return [] const sessionsById = new Map() for (const [storeDirectory, store] of stores.children) { - for (const session of store.getState().session) { + const state = store.getState() + for (const session of state.session) { const directory = session.directory || storeDirectory const current = sessionsById.get(session.id) if (!current || session.directory) sessionsById.set(session.id, { session, directory }) @@ -483,6 +510,9 @@ async function fetchSessionMessages(sessionId: string, directory?: string | null async function cascadeRevertToDescendants(rootId: string, cutoff: number): Promise { for (const { session, directory } of getDescendantSessions(rootId)) { try { + // A running descendant would keep writing messages past the revert + // boundary, so stop it first for the same reason the parent is aborted. + await abortDescendantIfBusy(session.id, directory) const messages = await fetchSessionMessages(session.id, directory) // Equal timestamps belong to the reverted side of the boundary. Keeping // them would rely on unrelated message IDs to decide chronology. @@ -500,6 +530,9 @@ async function cascadeUnrevertToDescendants(rootId: string): Promise { for (const { session, directory } of getDescendantSessions(rootId)) { if (!session.revert) continue try { + // Same reason as the revert cascade: a running descendant keeps writing + // messages that the unrevert would race against. + await abortDescendantIfBusy(session.id, directory) const result = await sdk().session.unrevert({ sessionID: session.id, directory }) mirrorSessionIntoLiveStores(assertSdkData(result, "session.unrevert"), directory) } catch (error) { diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index 24afa2ed..32ab4e1f 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] +- GitHub Copilot usage now shows a single AI Credits window, matching Copilot's token-based quota, in place of the old Chat Requests and Completions windows (thanks to @jakoss). - The context usage readout now reports the session cost including everything its subagents spent, matching the work status panel instead of showing a lower figure. ## [1.21.0] - 2026-08-26 diff --git a/packages/vscode/src/opencode.ts b/packages/vscode/src/opencode.ts index 14886b12..0fb47af3 100644 --- a/packages/vscode/src/opencode.ts +++ b/packages/vscode/src/opencode.ts @@ -686,7 +686,7 @@ async function spawnManagedOpenCodeServer( workingDirectory: string, port: number, timeoutMs: number -): Promise<{ url: string; close: () => void }> { +): Promise<{ url: string; close: () => Promise }> { const binary = stripWrappingQuotes(process.env.OPENCODE_BINARY || 'opencode') || 'opencode'; const launch = resolveWindowsLaunchSpec(binary, ['serve', '--hostname', '127.0.0.1', '--port', String(port)]); const child = spawn(launch.binary, launch.args, { @@ -761,18 +761,28 @@ async function spawnManagedOpenCodeServer( }); // Record this child so a future run can reap it if we crash before teardown. - registerManagedProcess({ pid: child.pid, ownerPid: process.pid, port, binary, runtime: 'vscode' }); + const registration = registerManagedProcess({ + pid: child.pid, + ownerPid: process.pid, + port, + binary, + runtime: 'vscode', + }).catch(() => {}); return { url, - close: () => { + close: async () => { killProcessTree(child.pid); try { child.kill('SIGTERM'); } catch { // ignore } - unregisterManagedProcess(child.pid); + // Both writes touch the same registry file. Unordered, the removal can + // land before the registration and leave a stale entry pointing at a dead + // pid; awaiting keeps the extension host alive until the file is gone. + await registration; + await unregisterManagedProcess(child.pid).catch(() => {}); }, }; } @@ -802,7 +812,7 @@ async function allocateManagedOpenCodePort(): Promise { } export function createOpenCodeManager(context: vscode.ExtensionContext): OpenCodeManager { - let server: { url: string; close: () => void } | null = null; + let server: { url: string; close: () => Promise } | null = null; let reapedOrphansOnce = false; let managedApiUrlOverride: string | null = null; let managedPassword: string | null = null; @@ -1017,7 +1027,7 @@ export function createOpenCodeManager(context: vscode.ExtensionContext): OpenCod setStatus('connected'); } else { try { - server.close(); + await server.close(); } catch { // ignore } @@ -1057,7 +1067,7 @@ export function createOpenCodeManager(context: vscode.ExtensionContext): OpenCod if (server) { try { - server.close(); + await server.close(); } catch { // Ignore close errors } diff --git a/packages/vscode/src/opencodeProcessRegistry.ts b/packages/vscode/src/opencodeProcessRegistry.ts index 903198c7..c11d5484 100644 --- a/packages/vscode/src/opencodeProcessRegistry.ts +++ b/packages/vscode/src/opencodeProcessRegistry.ts @@ -1,236 +1,16 @@ -// Managed OpenCode process registry + orphan reaper — VS Code parity copy. -// -// The VS Code extension does NOT bundle the web package, so it cannot import -// the web runtime's registry module. This is a parity implementation that -// reads/writes the SAME on-disk registry directory and uses the SAME algorithm, -// so a process spawned by any runtime (web, desktop, VS Code) can be reaped by -// any other. -// -// Storage is ONE FILE PER SPAWNED PROCESS (`.json`) in a registry -// directory — never a single shared JSON file — because multiple runtimes and -// windows run concurrently and a shared file would be clobbered by the -// read-modify-write race. Per-process files mean each instance only ever writes -// or deletes its OWN file. -// -// See packages/web/server/lib/opencode/managed-process-registry.js for the full -// rationale and safety model. In short: we only ever kill pids THIS product -// recorded, re-verified as a live `opencode serve`, and only when their spawner -// is provably gone (reparented to pid 1, or recorded owner pid dead). - -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; -import { spawnSync } from 'node:child_process'; - -type ManagedProcessEntry = { - pid: number; - ownerPid: number; - port: number | null; - binary: string | null; - runtime: string; - startedAt: string; -}; - -const resolveRegistryDir = (): string => { - const override = process.env.OPENCHAMBER_MANAGED_PROCESS_REGISTRY; - if (override && override.trim()) return override.trim(); - return path.join(os.homedir(), '.config', 'openchamber', 'managed-opencode'); -}; - -const entryFilePath = (pid: number): string => path.join(resolveRegistryDir(), `${pid}.json`); - -const writeEntryFile = (entry: ManagedProcessEntry): void => { - const dir = resolveRegistryDir(); - try { - fs.mkdirSync(dir, { recursive: true }); - const filePath = path.join(dir, `${entry.pid}.json`); - const tmp = `${filePath}.tmp-${process.pid}`; - fs.writeFileSync(tmp, JSON.stringify(entry, null, 2)); - fs.renameSync(tmp, filePath); - } catch { - // Best-effort: a failed registry write must never break spawn/shutdown. - } -}; - -const readAllEntries = (): Array<{ entry: ManagedProcessEntry; filePath: string }> => { - const dir = resolveRegistryDir(); - let names: string[] = []; - try { - names = fs.readdirSync(dir).filter((name) => name.endsWith('.json')); - } catch { - return []; - } - const out: Array<{ entry: ManagedProcessEntry; filePath: string }> = []; - for (const name of names) { - const filePath = path.join(dir, name); - try { - const entry = JSON.parse(fs.readFileSync(filePath, 'utf8')); - if (entry && Number.isInteger(entry.pid)) { - out.push({ entry: entry as ManagedProcessEntry, filePath }); - } else { - fs.rmSync(filePath, { force: true }); - } - } catch { - try { fs.rmSync(filePath, { force: true }); } catch { /* ignore */ } - } - } - return out; -}; - -export const registerManagedProcess = (input: { - pid: number | undefined; - ownerPid?: number; - port?: number | null; - binary?: string | null; - runtime?: string; -}): void => { - const pid = input.pid; - if (!Number.isInteger(pid)) return; - writeEntryFile({ - pid: pid as number, - ownerPid: Number.isInteger(input.ownerPid) ? (input.ownerPid as number) : process.pid, - port: Number.isInteger(input.port as number) ? (input.port as number) : null, - binary: typeof input.binary === 'string' ? input.binary : null, - runtime: typeof input.runtime === 'string' ? input.runtime : 'vscode', - startedAt: new Date().toISOString(), - }); -}; - -export const unregisterManagedProcess = (pid: number | undefined): void => { - if (!Number.isInteger(pid)) return; - try { - fs.rmSync(entryFilePath(pid as number), { force: true }); - } catch { - // ignore - } -}; - -const isPidAlive = (pid: number): boolean => { - if (!Number.isInteger(pid)) return false; - try { - process.kill(pid, 0); - return true; - } catch (error) { - return (error as NodeJS.ErrnoException)?.code === 'EPERM'; - } -}; - -const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); - -const readUnixProcInfo = (pid: number): { ppid: number; command: string } | null => { - try { - const result = spawnSync('ps', ['-p', String(pid), '-o', 'ppid=,command='], { - encoding: 'utf8', - timeout: 3000, - windowsHide: true, - }); - const line = (result.stdout || '').trim(); - if (!line) return null; - const match = line.match(/^\s*(\d+)\s+(.*)$/); - if (!match) return null; - return { ppid: Number.parseInt(match[1], 10), command: match[2] }; - } catch { - return null; - } -}; - -const readWindowsImageName = (pid: number): string | null => { - try { - const result = spawnSync('tasklist', ['/FI', `PID eq ${pid}`, '/FO', 'CSV', '/NH'], { - encoding: 'utf8', - timeout: 3000, - windowsHide: true, - }); - return (result.stdout || '').trim() || null; - } catch { - return null; - } -}; - -const commandIdentifiesOurServer = (command: string, entry: ManagedProcessEntry): boolean => { - if (typeof command !== 'string') return false; - const lower = command.toLowerCase(); - if (!lower.includes('opencode') || !lower.includes('serve')) return false; - if (Number.isInteger(entry.port) && !command.includes(String(entry.port))) return false; - return true; -}; - -const killOrphan = async (pid: number): Promise => { - if (process.platform === 'win32') { - try { - spawnSync('taskkill', ['/PID', String(pid), '/T', '/F'], { stdio: 'ignore', timeout: 5000, windowsHide: true }); - } catch { - // ignore - } - return; - } - - const signalTree = (signal: NodeJS.Signals) => { - try { process.kill(-pid, signal); } catch { /* ignore */ } - try { process.kill(pid, signal); } catch { /* ignore */ } - }; - - signalTree('SIGTERM'); - for (let waited = 0; waited < 1500 && isPidAlive(pid); waited += 150) { - await sleep(150); - } - if (isPidAlive(pid)) { - signalTree('SIGKILL'); - await sleep(300); - } -}; - -const processEntry = async ( - entry: ManagedProcessEntry, - log?: (message: string) => void, -): Promise => { - if (!isPidAlive(entry.pid)) return false; - - const ownerGone = Number.isInteger(entry.ownerPid) && !isPidAlive(entry.ownerPid); - - if (process.platform === 'win32') { - const image = readWindowsImageName(entry.pid); - const looksLikeOpencode = typeof image === 'string' && image.toLowerCase().includes('opencode'); - if (looksLikeOpencode && ownerGone) { - await killOrphan(entry.pid); - log?.(`[opencode] reaped orphaned process pid ${entry.pid} (owner ${entry.ownerPid} gone)`); - return true; - } - return false; - } - - const info = readUnixProcInfo(entry.pid); - if (!info || !commandIdentifiesOurServer(info.command, entry)) return false; - - const orphaned = info.ppid === 1 || ownerGone; - if (!orphaned) return false; - - await killOrphan(entry.pid); - log?.(`[opencode] reaped orphaned process pid ${entry.pid} (reparented/owner gone)`); - return true; -}; - -export const reapOrphanedProcesses = async ( - options: { log?: (message: string) => void } = {}, -): Promise<{ inspected: number; reaped: number }> => { - const { log } = options; - const records = readAllEntries(); - if (records.length === 0) return { inspected: 0, reaped: 0 }; - - let reaped = 0; - for (const { entry, filePath } of records) { - let drop = false; - try { - const wasReaped = await processEntry(entry, log); - if (wasReaped) reaped += 1; - drop = wasReaped || !isPidAlive(entry.pid); - } catch (error) { - log?.(`[opencode] reap check failed for pid ${entry.pid}: ${error instanceof Error ? error.message : error}`); - } - if (drop) { - try { fs.rmSync(filePath, { force: true }); } catch { /* ignore */ } - } - } - - return { inspected: records.length, reaped }; -}; +/** + * Managed OpenCode process registry + orphan reaper. + * + * Shared with packages/web/server/lib/opencode/managed-process-registry.js via + * esbuild bundling. Keep this module as a thin re-export so web and VS Code + * cannot diverge: a process spawned by any runtime (web, desktop, VS Code) must + * be reapable by any other, which only holds while all runtimes read and write + * the same on-disk registry with the same algorithm. + * + * Callers here pass `runtime: 'vscode'`; the shared module defaults to 'web'. + */ +export { + registerManagedProcess, + unregisterManagedProcess, + reapOrphanedProcesses, +} from '../../web/server/lib/opencode/managed-process-registry.js'; diff --git a/packages/web/bin/lib/cli-settings-accessors.js b/packages/web/bin/lib/cli-settings-accessors.js index 50d415bd..148917d7 100644 --- a/packages/web/bin/lib/cli-settings-accessors.js +++ b/packages/web/bin/lib/cli-settings-accessors.js @@ -40,9 +40,17 @@ export const createSettingsAccessors = ({ fsPromises, path, dataDir, settingsFil } throw error; } - const parsed = JSON.parse(raw); + const corruptSettingsError = (cause) => + new Error(`Settings file is corrupt or unreadable: ${settingsPath} (fix or remove it, then retry)`, { cause }); + + let parsed; + try { + parsed = JSON.parse(raw); + } catch (error) { + throw corruptSettingsError(error); + } if (!parsed || typeof parsed !== 'object') { - throw new Error('Settings file is malformed (non-object payload)'); + throw corruptSettingsError(new Error('non-object payload')); } return parsed; }; diff --git a/packages/web/bin/lib/cli-settings-accessors.test.js b/packages/web/bin/lib/cli-settings-accessors.test.js index a48829fc..7c50d303 100644 --- a/packages/web/bin/lib/cli-settings-accessors.test.js +++ b/packages/web/bin/lib/cli-settings-accessors.test.js @@ -144,7 +144,16 @@ describe('cli settings accessors', () => { await withTempDir(async (dir) => { const accessors = makeAccessors(dir); fs.writeFileSync(path.join(dir, 'settings.json'), '"just a string"'); - await expect(accessors.readSettingsStrict()).rejects.toThrow(/non-object payload/); + await expect(accessors.readSettingsStrict()).rejects.toThrow(/corrupt or unreadable/); + }); + }); + + it('names the settings file in the strict read failure', async () => { + await withTempDir(async (dir) => { + const accessors = makeAccessors(dir); + const filePath = path.join(dir, 'settings.json'); + fs.writeFileSync(filePath, '{"unfinished": "trunc'); + await expect(accessors.readSettingsStrict()).rejects.toThrow(filePath); }); }); diff --git a/packages/web/server/lib/git/service.test.js b/packages/web/server/lib/git/service.test.js index 2dcfceba..9443fcb6 100644 --- a/packages/web/server/lib/git/service.test.js +++ b/packages/web/server/lib/git/service.test.js @@ -2,7 +2,7 @@ import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterAll, afterEach, describe, expect, it, vi } from 'vitest'; import simpleGit from 'simple-git'; import { @@ -480,6 +480,10 @@ describe('getWorktrees', () => { warnSpy.mockClear(); }); + afterAll(() => { + warnSpy.mockRestore(); + }); + it('returns an empty list for a non-git directory without warning', async () => { const nonGit = createTempDir(); diff --git a/packages/web/server/lib/opencode/managed-process-registry.d.ts b/packages/web/server/lib/opencode/managed-process-registry.d.ts new file mode 100644 index 00000000..d22454b8 --- /dev/null +++ b/packages/web/server/lib/opencode/managed-process-registry.d.ts @@ -0,0 +1,13 @@ +export function registerManagedProcess(entry: { + pid?: number; + ownerPid?: number; + port?: number | null; + binary?: string | null; + runtime?: string; +}): Promise; + +export function unregisterManagedProcess(pid?: number): Promise; + +export function reapOrphanedProcesses(options?: { + log?: (message: string) => void; +}): Promise<{ inspected: number; reaped: number }>; diff --git a/packages/web/server/lib/opencode/managed-process-registry.js b/packages/web/server/lib/opencode/managed-process-registry.js index a6b6927b..f60d7378 100644 --- a/packages/web/server/lib/opencode/managed-process-registry.js +++ b/packages/web/server/lib/opencode/managed-process-registry.js @@ -51,7 +51,7 @@ import path from 'node:path'; import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; -const execFileAsync = promisify(execFile); +const defaultExecFileAsync = promisify(execFile); const resolveRegistryDir = () => { const override = process.env.OPENCHAMBER_MANAGED_PROCESS_REGISTRY; @@ -61,72 +61,6 @@ const resolveRegistryDir = () => { const entryFilePath = (pid) => path.join(resolveRegistryDir(), `${pid}.json`); -const writeEntryFile = async (entry) => { - const dir = resolveRegistryDir(); - try { - await fsp.mkdir(dir, { recursive: true }); - const filePath = path.join(dir, `${entry.pid}.json`); - const tmp = `${filePath}.tmp-${process.pid}`; - await fsp.writeFile(tmp, JSON.stringify(entry, null, 2)); - await fsp.rename(tmp, filePath); - } catch { - // Best-effort: a failed registry write must never break spawn/shutdown. - } -}; - -const readAllEntries = async () => { - const dir = resolveRegistryDir(); - let names = []; - try { - names = await fsp.readdir(dir); - } catch { - return []; - } - const out = []; - for (const name of names.filter((value) => value.endsWith('.json'))) { - const filePath = path.join(dir, name); - try { - const entry = JSON.parse(await fsp.readFile(filePath, 'utf8')); - if (entry && Number.isInteger(entry.pid)) { - out.push({ entry, filePath }); - } else { - await fsp.rm(filePath, { force: true }); - } - } catch { - // Corrupt/partial file — drop it. - try { - await fsp.rm(filePath, { force: true }); - } catch { - // ignore - } - } - } - return out; -}; - -/** Record an OpenCode process WE spawned so a future run can reap it if orphaned. */ -export const registerManagedProcess = async ({ pid, ownerPid, port, binary, runtime } = {}) => { - if (!Number.isInteger(pid)) return; - await writeEntryFile({ - pid, - ownerPid: Number.isInteger(ownerPid) ? ownerPid : process.pid, - port: Number.isInteger(port) ? port : null, - binary: typeof binary === 'string' ? binary : null, - runtime: typeof runtime === 'string' ? runtime : 'web', - startedAt: new Date().toISOString(), - }); -}; - -/** Drop a pid from the registry (after we have killed/closed it ourselves). */ -export const unregisterManagedProcess = async (pid) => { - if (!Number.isInteger(pid)) return; - try { - await fsp.rm(entryFilePath(pid), { force: true }); - } catch { - // Best-effort: dropping a missing file is not an error. - } -}; - const isPidAlive = (pid) => { if (!Number.isInteger(pid)) return false; try { @@ -140,38 +74,6 @@ const isPidAlive = (pid) => { const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); -// Returns { ppid, command } for a live pid on Unix, or null if it can't be read. -const readUnixProcInfo = async (pid) => { - try { - const { stdout } = await execFileAsync('ps', ['-p', String(pid), '-o', 'ppid=,command='], { - encoding: 'utf8', - timeout: 3000, - windowsHide: true, - }); - const line = (stdout || '').trim(); - if (!line) return null; - const match = line.match(/^\s*(\d+)\s+(.*)$/); - if (!match) return null; - return { ppid: Number.parseInt(match[1], 10), command: match[2] }; - } catch { - return null; - } -}; - -// Windows image name for a pid (e.g. "opencode.exe"), or null. -const readWindowsImageName = async (pid) => { - try { - const { stdout } = await execFileAsync('tasklist', ['/FI', `PID eq ${pid}`, '/FO', 'CSV', '/NH'], { - encoding: 'utf8', - timeout: 3000, - windowsHide: true, - }); - return (stdout || '').trim() || null; - } catch { - return null; - } -}; - const commandIdentifiesOurServer = (command, entry) => { if (typeof command !== 'string') return false; const lower = command.toLowerCase(); @@ -182,105 +84,218 @@ const commandIdentifiesOurServer = (command, entry) => { return true; }; -const killOrphan = async (pid) => { - if (process.platform === 'win32') { +/** + * Build the registry API over injectable filesystem and child-process + * dependencies. Production callers use the default instance exported below; + * tests pass their own `fs`/`execFileAsync` instead of mocking node builtins. + */ +export const createManagedProcessRegistry = ({ fs = fsp, execFileAsync = defaultExecFileAsync } = {}) => { + const writeEntryFile = async (entry) => { + const dir = resolveRegistryDir(); try { - await execFileAsync('taskkill', ['/PID', String(pid), '/T', '/F'], { - stdio: 'ignore', - timeout: 5000, - windowsHide: true, - }); + await fs.mkdir(dir, { recursive: true }); + const filePath = path.join(dir, `${entry.pid}.json`); + const tmp = `${filePath}.tmp-${process.pid}`; + await fs.writeFile(tmp, JSON.stringify(entry, null, 2)); + await fs.rename(tmp, filePath); } catch { - // Best-effort: a failed kill is not fatal (startup reaper is a backstop). - } - return; - } - - const signalTree = (signal) => { - try { - process.kill(-pid, signal); - } catch { - // process group may already be gone - } - try { - process.kill(pid, signal); - } catch { - // pid may already be gone + // Best-effort: a failed registry write must never break spawn/shutdown. } }; - signalTree('SIGTERM'); - for (let waited = 0; waited < 1500 && isPidAlive(pid); waited += 150) { - await sleep(150); - } - if (isPidAlive(pid)) { - signalTree('SIGKILL'); - await sleep(300); - } -}; - -// Decide+act on a single registry entry. Returns true if it was reaped. -const processEntry = async (entry, { log }) => { - // Dead pid → nothing to do (caller drops the file). - if (!isPidAlive(entry.pid)) return false; - - const ownerGone = Number.isInteger(entry.ownerPid) && !isPidAlive(entry.ownerPid); - - if (process.platform === 'win32') { - const image = await readWindowsImageName(entry.pid); - const looksLikeOpencode = typeof image === 'string' && image.toLowerCase().includes('opencode'); - // Windows lacks reliable reparent-to-1 semantics (job objects usually kill - // children with the parent), so we reap only when the owner is provably dead - // AND the image still looks like opencode. - if (looksLikeOpencode && ownerGone) { - await killOrphan(entry.pid); - log?.(`[lifecycle] reaped orphaned OpenCode pid ${entry.pid} (owner ${entry.ownerPid} gone)`); - return true; - } - return false; - } - - const info = await readUnixProcInfo(entry.pid); - // Can't verify identity (or it's not our server) → leave it alone. - if (!info || !commandIdentifiesOurServer(info.command, entry)) return false; - - const orphaned = info.ppid === 1 || ownerGone; - if (!orphaned) return false; // still owned by a live instance - - await killOrphan(entry.pid); - log?.(`[lifecycle] reaped orphaned OpenCode pid ${entry.pid} (reparented/owner gone)`); - return true; -}; - -/** - * Kill any genuinely-orphaned OpenCode processes WE previously spawned, and - * prune their registry files. Safe to call at startup before spawning a new - * server. Returns { inspected, reaped }. - */ -export const reapOrphanedProcesses = async ({ log } = {}) => { - const records = await readAllEntries(); - if (records.length === 0) return { inspected: 0, reaped: 0 }; - - let reaped = 0; - for (const { entry, filePath } of records) { - let drop = false; + const readAllEntries = async () => { + const dir = resolveRegistryDir(); + let names = []; try { - const wasReaped = await processEntry(entry, { log }); - if (wasReaped) reaped += 1; - // Drop the file when the process is gone (reaped now, or already dead); - // keep it only while the process is still alive and owned by a live owner. - drop = wasReaped || !isPidAlive(entry.pid); - } catch (error) { - log?.(`[lifecycle] reap check failed for pid ${entry.pid}: ${error?.message ?? error}`); + names = await fs.readdir(dir); + } catch { + return []; } - if (drop) { + const out = []; + for (const name of names.filter((value) => value.endsWith('.json'))) { + const filePath = path.join(dir, name); try { - await fsp.rm(filePath, { force: true }); + const entry = JSON.parse(await fs.readFile(filePath, 'utf8')); + if (entry && Number.isInteger(entry.pid)) { + out.push({ entry, filePath }); + } else { + await fs.rm(filePath, { force: true }); + } } catch { - // best-effort + // Corrupt/partial file — drop it. + try { + await fs.rm(filePath, { force: true }); + } catch { + // ignore + } } } - } + return out; + }; - return { inspected: records.length, reaped }; + /** Record an OpenCode process WE spawned so a future run can reap it if orphaned. */ + const registerManagedProcess = async ({ pid, ownerPid, port, binary, runtime } = {}) => { + if (!Number.isInteger(pid)) return; + await writeEntryFile({ + pid, + ownerPid: Number.isInteger(ownerPid) ? ownerPid : process.pid, + port: Number.isInteger(port) ? port : null, + binary: typeof binary === 'string' ? binary : null, + runtime: typeof runtime === 'string' ? runtime : 'web', + startedAt: new Date().toISOString(), + }); + }; + + /** Drop a pid from the registry (after we have killed/closed it ourselves). */ + const unregisterManagedProcess = async (pid) => { + if (!Number.isInteger(pid)) return; + try { + await fs.rm(entryFilePath(pid), { force: true }); + } catch { + // Best-effort: dropping a missing file is not an error. + } + }; + + // Returns { ppid, command } for a live pid on Unix, or null if it can't be read. + const readUnixProcInfo = async (pid) => { + try { + const { stdout } = await execFileAsync('ps', ['-p', String(pid), '-o', 'ppid=,command='], { + encoding: 'utf8', + timeout: 3000, + windowsHide: true, + }); + const line = (stdout || '').trim(); + if (!line) return null; + const match = line.match(/^\s*(\d+)\s+(.*)$/); + if (!match) return null; + return { ppid: Number.parseInt(match[1], 10), command: match[2] }; + } catch { + return null; + } + }; + + // Windows image name for a pid (e.g. "opencode.exe"), or null. + const readWindowsImageName = async (pid) => { + try { + const { stdout } = await execFileAsync('tasklist', ['/FI', `PID eq ${pid}`, '/FO', 'CSV', '/NH'], { + encoding: 'utf8', + timeout: 3000, + windowsHide: true, + }); + return (stdout || '').trim() || null; + } catch { + return null; + } + }; + + const killOrphan = async (pid) => { + if (process.platform === 'win32') { + try { + await execFileAsync('taskkill', ['/PID', String(pid), '/T', '/F'], { + stdio: 'ignore', + timeout: 5000, + windowsHide: true, + }); + } catch { + // Best-effort: a failed kill is not fatal (startup reaper is a backstop). + } + return; + } + + const signalTree = (signal) => { + try { + process.kill(-pid, signal); + } catch { + // process group may already be gone + } + try { + process.kill(pid, signal); + } catch { + // pid may already be gone + } + }; + + signalTree('SIGTERM'); + for (let waited = 0; waited < 1500 && isPidAlive(pid); waited += 150) { + await sleep(150); + } + if (isPidAlive(pid)) { + signalTree('SIGKILL'); + await sleep(300); + } + }; + + // Decide+act on a single registry entry. Returns true if it was reaped. + const processEntry = async (entry, { log }) => { + // Dead pid → nothing to do (caller drops the file). + if (!isPidAlive(entry.pid)) return false; + + const ownerGone = Number.isInteger(entry.ownerPid) && !isPidAlive(entry.ownerPid); + + if (process.platform === 'win32') { + const image = await readWindowsImageName(entry.pid); + const looksLikeOpencode = typeof image === 'string' && image.toLowerCase().includes('opencode'); + // Windows lacks reliable reparent-to-1 semantics (job objects usually kill + // children with the parent), so we reap only when the owner is provably dead + // AND the image still looks like opencode. + if (looksLikeOpencode && ownerGone) { + await killOrphan(entry.pid); + log?.(`[lifecycle] reaped orphaned OpenCode pid ${entry.pid} (owner ${entry.ownerPid} gone)`); + return true; + } + return false; + } + + const info = await readUnixProcInfo(entry.pid); + // Can't verify identity (or it's not our server) → leave it alone. + if (!info || !commandIdentifiesOurServer(info.command, entry)) return false; + + const orphaned = info.ppid === 1 || ownerGone; + if (!orphaned) return false; // still owned by a live instance + + await killOrphan(entry.pid); + log?.(`[lifecycle] reaped orphaned OpenCode pid ${entry.pid} (reparented/owner gone)`); + return true; + }; + + /** + * Kill any genuinely-orphaned OpenCode processes WE previously spawned, and + * prune their registry files. Safe to call at startup before spawning a new + * server. Returns { inspected, reaped }. + */ + const reapOrphanedProcesses = async ({ log } = {}) => { + const records = await readAllEntries(); + if (records.length === 0) return { inspected: 0, reaped: 0 }; + + let reaped = 0; + for (const { entry, filePath } of records) { + let drop = false; + try { + const wasReaped = await processEntry(entry, { log }); + if (wasReaped) reaped += 1; + // Drop the file when the process is gone (reaped now, or already dead); + // keep it only while the process is still alive and owned by a live owner. + drop = wasReaped || !isPidAlive(entry.pid); + } catch (error) { + log?.(`[lifecycle] reap check failed for pid ${entry.pid}: ${error?.message ?? error}`); + } + if (drop) { + try { + await fs.rm(filePath, { force: true }); + } catch { + // best-effort + } + } + } + + return { inspected: records.length, reaped }; + }; + + return { registerManagedProcess, unregisterManagedProcess, reapOrphanedProcesses }; }; + +const defaultRegistry = createManagedProcessRegistry(); + +export const registerManagedProcess = defaultRegistry.registerManagedProcess; +export const unregisterManagedProcess = defaultRegistry.unregisterManagedProcess; +export const reapOrphanedProcesses = defaultRegistry.reapOrphanedProcesses; diff --git a/packages/web/server/lib/opencode/managed-process-registry.test.mjs b/packages/web/server/lib/opencode/managed-process-registry.test.mjs index 7f1891c7..73c9d703 100644 --- a/packages/web/server/lib/opencode/managed-process-registry.test.mjs +++ b/packages/web/server/lib/opencode/managed-process-registry.test.mjs @@ -1,18 +1,9 @@ -import { promisify } from 'node:util'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -// Mocks must be in place before the module under test is imported, because the -// module calls `promisify(execFile)` at module-load time and binds `fsp.*` at -// call time. -// -// NOTE on `promisify.custom`: the real `child_process.execFile` carries a -// `[util.promisify.custom]` symbol so that `promisify(execFile)` resolves to -// `{ stdout, stderr }` (not the generic multi-arg array). A plain `vi.fn()` -// mock lacks that symbol, so `const { stdout } = await execFileAsync(...)` -// would destructure `undefined`. We attach the symbol to the mock so the -// promisified helper used by the module resolves to the same `{ stdout, -// stderr }` shape. +import { createManagedProcessRegistry } from './managed-process-registry.js'; +// The registry takes its filesystem and child-process helpers as dependencies, +// so these tests inject fakes instead of mocking node builtins. const readdirMock = vi.fn(); const readFileMock = vi.fn(); const rmMock = vi.fn(); @@ -20,8 +11,13 @@ const mkdirMock = vi.fn(); const writeFileMock = vi.fn(); const renameMock = vi.fn(); -vi.mock('node:fs/promises', () => ({ - default: { +// `execFileImpl` is the swappable per-test implementation, called with the same +// (cmd, args, opts, cb) shape the callback-style `execFile` uses; the injected +// `execFileAsync` adapts it to the `{ stdout, stderr }` promise the module awaits. +const execFileImpl = vi.fn(); + +const { registerManagedProcess, unregisterManagedProcess, reapOrphanedProcesses } = createManagedProcessRegistry({ + fs: { readdir: readdirMock, readFile: readFileMock, rm: rmMock, @@ -29,29 +25,12 @@ vi.mock('node:fs/promises', () => ({ writeFile: writeFileMock, rename: renameMock, }, -})); - -// `execFileImpl` is the swappable per-test implementation; `execFileMock` is -// what the mocked module sees. `promisify(execFileMock)` returns the custom -// function, which delegates to `execFileImpl` with a (err, stdout, stderr) -// callback and resolves to `{ stdout, stderr }`. -const execFileImpl = vi.fn(); -const execFileMock = vi.fn(); -execFileMock[promisify.custom] = (cmd, args, opts) => - new Promise((resolve, reject) => { - execFileImpl(cmd, args, opts, (err, stdout, stderr) => - err ? reject(err) : resolve({ stdout: stdout ?? '', stderr: stderr ?? '' })); - }); - -vi.mock('node:child_process', () => ({ - execFile: execFileMock, -})); - -const { - registerManagedProcess, - unregisterManagedProcess, - reapOrphanedProcesses, -} = await import('./managed-process-registry.js'); + execFileAsync: (cmd, args, opts) => + new Promise((resolve, reject) => { + execFileImpl(cmd, args, opts, (err, stdout, stderr) => + err ? reject(err) : resolve({ stdout: stdout ?? '', stderr: stderr ?? '' })); + }), +}); const ORIGINAL_PLATFORM = Object.getOwnPropertyDescriptor(process, 'platform'); const ORIGINAL_KILL = process.kill;