From 378c60071a85450dfc844c5722eda61a132df6a1 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 16 Aug 2026 20:33:37 -0700 Subject: [PATCH 1/8] fix(grok): strip quotes from GROK_HOME in the Windows hook (#14221) (#14985) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setx GROK_HOME "C:\path\"` stores `C:\path"` — the CRT turns the `\"` into a literal quote. That quote unbalanced the trailing-backslash `if` operand, so cmd aborted grok-hook.cmd with exit 255 before curl and every Grok hook event failed. A quote elsewhere in the value closed curl's `grokHome=` argument early, swallowing the `^` continuation and dropping the `payload@-` line. `"` is illegal in a Windows path, so strip it during the copy. Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- src/main/grok/hook-service.test.ts | 4 +- .../grok/windows-grok-hook-script.test.ts | 39 +++++++++++++++++-- src/main/grok/windows-grok-hook-script.ts | 6 ++- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/main/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index ba469506316..9a58aabaf24 100644 --- a/src/main/grok/hook-service.test.ts +++ b/src/main/grok/hook-service.test.ts @@ -118,7 +118,7 @@ describe('GrokHookService', () => { expect(script).toContain('set "ORCA_GROK_HOME="') expect(script).toContain('if not defined GROK_HOME goto :orca_grok_home_ready') expect(script).toContain('%GROK_HOME:~4096,1%') - expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME:"=%"') expect(script).toContain('%ORCA_GROK_HOME:~4096,1%') expect(script).toContain(':orca_grok_home_ready') expect(script).toContain('if not defined ORCA_GROK_HOME goto :orca_grok_home_ready') @@ -269,7 +269,7 @@ describe('GrokHookService', () => { expect(script).toContain('%SystemRoot%\\System32\\curl.exe') // Why: windows-grok-hook-script.test.ts pins the GROK_HOME guard shape itself, // and does so on every platform rather than only on Windows runners. - expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME%"') + expect(script).toContain('set "ORCA_GROK_HOME=%GROK_HOME:"=%"') expect(script).toContain('--data-urlencode "grokHome=%ORCA_GROK_HOME%"') } else { // Why: payload is piped to curl via stdin (`payload@-`) so it never lands diff --git a/src/main/grok/windows-grok-hook-script.test.ts b/src/main/grok/windows-grok-hook-script.test.ts index a8aea9065f0..98da524e7c7 100644 --- a/src/main/grok/windows-grok-hook-script.test.ts +++ b/src/main/grok/windows-grok-hook-script.test.ts @@ -26,8 +26,9 @@ function findUnguardedSubstringReads(lines: readonly string[]): string[] { defined.add(guarded[1]) } // A value is provably non-empty only if something outside an expansion survives: - // `set "V=%OTHER%"` may undefine V, `set "V=%V%."` cannot. - for (const [, name, value] of line.matchAll(/set "(\w+)=([^"]*)"/g)) { + // `set "V=%OTHER%"` may undefine V, `set "V=%V%."` cannot. The lazy value stops at + // the quote that closes the `set`, so a substitution like `%V:"=%` stays one token. + for (const [, name, value] of line.matchAll(/set "(\w+)=(.*?)"(?=\s|$)/g)) { if (value.replaceAll(/%[^%]*%/g, '') === '') { defined.delete(name) } else { @@ -93,6 +94,7 @@ describe.skipIf(process.platform !== 'win32')('buildWindowsGrokHookScript (win32 grokHome: string | null paneKey: string | null worktreeId: string | null + payload: string | null } const PANE_KEY = 'tab!1:pane!2' @@ -149,7 +151,8 @@ describe.skipIf(process.platform !== 'win32')('buildWindowsGrokHookScript (win32 stderr, grokHome: posted?.get('grokHome') ?? null, paneKey: posted?.get('paneKey') ?? null, - worktreeId: posted?.get('worktreeId') ?? null + worktreeId: posted?.get('worktreeId') ?? null, + payload: posted?.get('payload') ?? null }) ) }) @@ -188,6 +191,36 @@ describe.skipIf(process.platform !== 'win32')('buildWindowsGrokHookScript (win32 expect(result.grokHome).toBe('C:\\Users\\test\\.grok\\.') }) + // Why (#14221): `setx GROK_HOME "C:\path\"` stores `C:\path"` — the CRT turns the + // `\"` into a literal quote. That quote unbalanced the trailing-backslash `if`, so + // cmd aborted the whole script before curl and every Grok hook event failed. + it('exits 0 and strips a trailing quote from GROK_HOME', async () => { + const result = await runHook('C:\\Users\\test\\.grok"') + + expect(result.stderr).not.toContain('unexpected at this time') + expect(result.status).toBe(0) + expect(result.grokHome).toBe('C:\\Users\\test\\.grok') + expect(result.payload).toBe('{"session_id":"s"}') + }) + + // Why: an embedded quote closed curl's `grokHome=` argument early, which swallowed + // the `^` continuation and dropped the `payload@-` line — a silent, exit-0 failure. + it('strips an embedded quote without truncating the curl arguments', async () => { + const result = await runHook('C:\\Users\\a"b\\.grok') + + expect(result.status).toBe(0) + expect(result.grokHome).toBe('C:\\Users\\ab\\.grok') + expect(result.payload).toBe('{"session_id":"s"}') + }) + + it('posts an empty grokHome when GROK_HOME is only a quote', async () => { + const result = await runHook('"') + + expect(result.status).toBe(0) + expect(result.grokHome).toBe('') + expect(result.payload).toBe('{"session_id":"s"}') + }) + it('drops a GROK_HOME past the envelope limit', async () => { const result = await runHook(`C:\\${'a'.repeat(4094)}`) diff --git a/src/main/grok/windows-grok-hook-script.ts b/src/main/grok/windows-grok-hook-script.ts index 5a6e81ee633..e68c8914eeb 100644 --- a/src/main/grok/windows-grok-hook-script.ts +++ b/src/main/grok/windows-grok-hook-script.ts @@ -21,6 +21,7 @@ const WINDOWS_GROK_HOOK_POST_COMMAND = buildWindowsAgentHookPostCommand('grok', * * - Guard substring ops behind `if defined` + goto (not a parenthesized block). * - Reject oversized values before copying them onto a cmd input line. + * - Strip `"` before the value reaches an `if` operand or the curl argument. */ export function buildWindowsGrokHookScript(): string { return [ @@ -35,7 +36,10 @@ export function buildWindowsGrokHookScript(): string { 'set "ORCA_GROK_HOME="', 'if not defined GROK_HOME goto :orca_grok_home_ready', `if not "%GROK_HOME:~${GROK_HOME_ENVELOPE_MAX_LENGTH},1%"=="" goto :orca_grok_home_ready`, - 'set "ORCA_GROK_HOME=%GROK_HOME%"', + // Why (#14221): `setx GROK_HOME "C:\path\"` stores a literal trailing quote, which + // unbalances the trailing-backslash `if` below (exit 255) and truncates the curl + // argument. `"` is illegal in a Windows path, so strip it rather than preserve it. + 'set "ORCA_GROK_HOME=%GROK_HOME:"=%"', 'if not defined ORCA_GROK_HOME goto :orca_grok_home_ready', 'if "%ORCA_GROK_HOME:~-1%"=="\\" set "ORCA_GROK_HOME=%ORCA_GROK_HOME%."', // Why: the trailing-backslash safety sentinel counts toward the relay envelope. From e16a22ef586dd4329a8a80076f0cc2a1f0d22d8b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:35:34 -0700 Subject: [PATCH 2/8] fix(browser): return a real PermissionStatus from the query override (#14684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(browser): shadow PermissionStatus state instead of proxying it Rebased onto main after #14685 landed in the same file. Both changes coexist: #14685's Firefox gating and narrowed promptPerms are preserved untouched, and this change replaces only the query-override implementation. The previous Proxy bound every callable property, which broke three observable things: - onchange assignment threw "Illegal invocation" — the native setter received the proxy rather than the branded target. - Listeners were delivered with the real target and the NATIVE state, so on a real permission change event.target.state read 'granted' while the returned status still read 'prompt'. - Method reads returned a new bound function each time, so name became 'bound addEventListener', toString lost its name, and identity was unstable. Separately, the notifications state was captured once as a string, so an already-returned status went stale after requestPermission updated it. Shadow only 'state' on the genuine PermissionStatus with a lazy provider. The object the site holds IS the real one, so identity, brand checks, method fingerprints and native event delivery survive with nothing to keep in sync. Verified against real Chromium (Chrome 151) rather than only the vm stand-in, because the stand-in cannot show whether defineProperty succeeds on a branded instance: the instance is extensible, the prototype 'state' accessor is configurable, defineProperty succeeds, instanceof survives, addEventListener keeps its native name and referential stability, and onchange assignment works. New tests live in their own file rather than merged into anti-detection.test.ts, whose harness diverged on main. The fallback test targets 'camera' because #14685 narrowed the intercepted set, so a name outside it never reaches the fallback. * test(browser): cover intercepted PermissionStatus identity --- .../anti-detection-permission-status.test.ts | 210 ++++++++++++++++++ src/main/browser/anti-detection.ts | 35 ++- 2 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 src/main/browser/anti-detection-permission-status.test.ts diff --git a/src/main/browser/anti-detection-permission-status.test.ts b/src/main/browser/anti-detection-permission-status.test.ts new file mode 100644 index 00000000000..d686e32c975 --- /dev/null +++ b/src/main/browser/anti-detection-permission-status.test.ts @@ -0,0 +1,210 @@ +import { runInNewContext } from 'node:vm' +import { describe, expect, it } from 'vitest' + +import { ANTI_DETECTION_SCRIPT } from './anti-detection' + +type PermissionQueryResult = EventTarget & { + state: string + onchange: EventListener | null + marker: string +} + +type PermissionStatusConstructor = { + new (): PermissionQueryResult + prototype: PermissionQueryResult +} + +type AntiDetectionContext = { + Notification: { + permission: string + requestPermission: (callback?: (permission: string) => void) => Promise + } + PermissionStatus: PermissionStatusConstructor + dispatchPermissionChange: (name: string) => void + navigator: { + permissions: { + query: (descriptor: { name: string }) => Promise + } + } +} + +function createContext(args: { + nativeNotificationPermission: string + requestedNotificationPermission: string + rejectedPermissions?: string[] +}): AntiDetectionContext & Record { + class PermissionStatus extends EventTarget { + #state = 'denied' + #onchange: EventListener | null = null + marker = 'real-status' + + get state(): string { + return this.#state + } + + get onchange(): EventListener | null { + return this.#onchange + } + + set onchange(listener: EventListener | null) { + if (this.#onchange) { + super.removeEventListener('change', this.#onchange) + } + this.#onchange = typeof listener === 'function' ? listener : null + if (this.#onchange) { + super.addEventListener('change', this.#onchange) + } + } + } + + const statuses = new Map() + const rejectedPermissions = new Set(args.rejectedPermissions) + + class Permissions { + query(descriptor: { name: string }): Promise { + if (rejectedPermissions.has(descriptor.name)) { + return Promise.reject(new Error('Unsupported permission')) + } + const status = new PermissionStatus() + const permissionStatuses = statuses.get(descriptor.name) ?? [] + permissionStatuses.push(status) + statuses.set(descriptor.name, permissionStatuses) + return Promise.resolve(status) + } + } + + const Notification = { + permission: args.nativeNotificationPermission, + requestPermission(callback?: (permission: string) => void): Promise { + callback?.(args.requestedNotificationPermission) + return Promise.resolve(args.requestedNotificationPermission) + } + } + Object.defineProperty(Notification, 'permission', { + configurable: true, + get: () => args.nativeNotificationPermission + }) + + return { + Date, + Event, + EventTarget, + Object, + Promise, + Set, + performance: { now: () => 0 }, + // Why: the script's Firefox gate reads navigator.userAgent, so a non-Firefox UA keeps these + // tests on the ordinary-page path where the PermissionStatus override applies. + window: { chrome: {} }, + navigator: { + userAgent: + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/150.0.0.0 Safari/537.36', + plugins: [], + languages: [], + permissions: new Permissions() + }, + Permissions, + PermissionStatus, + Notification, + dispatchPermissionChange(name: string): void { + for (const status of statuses.get(name) ?? []) { + status.dispatchEvent(new Event('change')) + } + } + } as AntiDetectionContext & Record +} + +describe('ANTI_DETECTION_SCRIPT — PermissionStatus', () => { + it('keeps an existing notification status current after permission changes', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted' + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'notifications' }) + + expect(context.Notification.permission).toBe('default') + expect(status.state).toBe('prompt') + + await expect(context.Notification.requestPermission()).resolves.toBe('granted') + + expect(context.Notification.permission).toBe('granted') + expect(status.state).toBe('granted') + }) + + it('preserves native PermissionStatus identity and methods', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted' + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'camera' }) + const expectedSource = Function.prototype.toString.call( + context.PermissionStatus.prototype.addEventListener + ) + + expect(status).toBeInstanceOf(context.PermissionStatus) + expect(status.state).toBe('prompt') + expect(status.constructor.name).toBe('PermissionStatus') + expect(status.marker).toBe('real-status') + expect(status.addEventListener.name).toBe('addEventListener') + expect(status.addEventListener).toBe(status.addEventListener) + expect(Function.prototype.toString.call(status.addEventListener)).toBe(expectedSource) + expect(expectedSource).toContain('addEventListener') + }) + + it('delivers change events through the returned status with the overridden state', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted' + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'notifications' }) + const events: { receiver: EventTarget; target: EventTarget | null; state: string }[] = [] + const recordEvent = function (this: EventTarget, event: Event): void { + events.push({ + receiver: this, + target: event.target, + state: (event.target as PermissionQueryResult).state + }) + } + + status.addEventListener('change', recordEvent) + expect(() => { + status.onchange = function (this: EventTarget, event): void { + recordEvent.call(this, event) + } + }).not.toThrow() + + await context.Notification.requestPermission() + context.dispatchPermissionChange('notifications') + + expect(events).toHaveLength(2) + expect(events).toEqual([ + { receiver: status, target: status, state: 'granted' }, + { receiver: status, target: status, state: 'granted' } + ]) + }) + + // Why: 'camera' rather than 'storage-access' — #14685 narrowed the intercepted set to + // camera/microphone, so a name outside it falls through to the real query and never reaches + // the fallback at all. + it('uses a non-enumerable EventTarget fallback when the native query rejects', async () => { + const context = createContext({ + nativeNotificationPermission: 'denied', + requestedNotificationPermission: 'granted', + rejectedPermissions: ['camera'] + }) + + runInNewContext(ANTI_DETECTION_SCRIPT, context) + const status = await context.navigator.permissions.query({ name: 'camera' }) + + expect(status).toBeInstanceOf(EventTarget) + expect(status).not.toBeInstanceOf(context.PermissionStatus) + expect(status.state).toBe('prompt') + expect(Object.keys(status)).toEqual([]) + }) +}) diff --git a/src/main/browser/anti-detection.ts b/src/main/browser/anti-detection.ts index b955b200229..d725fbf965f 100644 --- a/src/main/browser/anti-detection.ts +++ b/src/main/browser/anti-detection.ts @@ -86,12 +86,43 @@ export const ANTI_DETECTION_SCRIPT = `(function() { 'camera', 'microphone' ]); const origQuery = Permissions.prototype.query; + // Why: sites must receive the genuine PermissionStatus so native events, brand checks and method + // identity survive. Shadow only state, and resolve it lazily so existing statuses stay current. + function withOverriddenState(realStatus, stateProvider) { + Object.defineProperty(realStatus, 'state', { + configurable: true, + get: stateProvider + }); + return realStatus; + } + // Why: some names the real implementation rejects outright; fall back to an EventTarget so + // listener registration still works instead of throwing. + function fallbackStatus(stateProvider) { + const status = new EventTarget(); + Object.defineProperties(status, { + state: { configurable: true, get: stateProvider }, + onchange: { configurable: true, value: null, writable: true } + }); + return status; + } + function queryWithState(permissions, desc, stateProvider) { + let real; + try { + real = origQuery.call(permissions, desc); + } catch { + return Promise.resolve(fallbackStatus(stateProvider)); + } + return Promise.resolve(real).then( + (status) => withOverriddenState(status, stateProvider), + () => fallbackStatus(stateProvider) + ); + } Permissions.prototype.query = function(desc) { if (desc.name === 'notifications') { - return Promise.resolve({ state: notificationPermissionState(), onchange: null }); + return queryWithState(this, desc, notificationPermissionState); } if (promptPerms.has(desc.name)) { - return Promise.resolve({ state: 'prompt', onchange: null }); + return queryWithState(this, desc, () => 'prompt'); } return origQuery.call(this, desc); }; From 5e9e38fa75d694fe5b59ad5b88d5f829f2bf36e6 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:47:02 -0700 Subject: [PATCH 3/8] fix(agent-status): announce Claude turn complete while background work runs (#14580) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agent-status): announce Claude turn complete while background work runs Lead Stop/StopFailure already ends the turn, but resolveClaudePaneState keeps the pane working for subagents, background shells, and session crons. That erases the working→done edge that mints the completion banner: subagent turns notify late with a stale body, and shells/crons never notify at all. Stamp turnCompletedAt on the gated lead Stop, announce immediately from that row, and pair the later all-clear done to the same end time so it cannot double-fire or collapse consecutive turns onto the pinned stateStartedAt. Fixes #13245 * fix(agent-status): suppress stamped turn replays * fix(agent-status): notify paired clients at turn end * Fix late-paired completion notification arming * test(agent-status): make the notification-id test fail on the pre-fix ordering The stored row inherited the helper's default codex agentType while the event named claude, so agentSnapshotMatchesExplicitTitle dropped it and freshStoredAgentStatus was undefined — the assertion held under either side of the `??`. Name the stored row's agent so the pinned working row survives and the snapshot-first precedence is what the test actually pins. * Suppress stamped completion tail replays * Prevent cross-coordinator title replays * Preserve stamped tails across fallback signals * Keep remount replay state while sibling lives * Scope OSC turn stamp preservation * Forward paired host completion stamps * Deduplicate paired completion tails * Bind paired completion tails to their turn * Preserve paired completion tail ownership * Keep paired tail replay state across remounts * Seed paired recovery without replaying completions * Seed startup replay and release stale fallback dedupe * fix(notifications): preserve stamped OSC repaints * fix(notifications): retain paired client turn boundary * fix notifications module import safety * chore: keep main integration focused * fix: preserve completion re-enable boundary --- ...k-status-session-tabs-invalidation.test.ts | 16 + .../hook-status-session-tabs-invalidation.ts | 4 +- .../server-ingest-terminal-status.test.ts | 91 ++++ src/main/agent-hooks/server.ts | 44 +- ...ntime-hook-agent-status-projection.test.ts | 17 + src/main/runtime/orca-runtime.ts | 39 +- ...ompletion-coordinator-dispose-leak.test.ts | 72 +++ ...-coordinator-hook-title-precedence.test.ts | 11 + ...-coordinator-stamped-turn-boundary.test.ts | 409 ++++++++++++++++++ ...on-coordinator-stamped-turn-replay.test.ts | 388 +++++++++++++++++ .../agent-completion-coordinator-types.ts | 4 + .../agent-completion-coordinator.ts | 351 ++++++++++++++- .../agent-completion-snapshot-staleness.ts | 12 +- .../terminal-pane/pty-connection.ts | 1 + .../use-notification-dispatch.test.ts | 93 ++++ .../use-notification-dispatch.ts | 2 +- ...tion-background-turn-notifications.test.ts | 270 ++++++++++++ ...gent-hook-completion-fresh-working.test.ts | 122 ++++++ ...ok-completion-notifications-import.test.ts | 17 + ...gent-hook-completion-notifications.test.ts | 5 +- .../agent-hook-completion-notifications.ts | 52 ++- .../hooks/agent-hook-completion-store-sync.ts | 8 +- ...events-agent-status-store-test-fixtures.ts | 1 + ...vents-agent-status-turn-completion.test.ts | 92 ++++ src/renderer/src/hooks/useIpcEvents.ts | 6 +- .../src/runtime/web-runtime-session.ts | 2 +- ...abs-agent-completion-notifications.test.ts | 325 ++++++++++++++ .../src/runtime/web-session-tabs-sync.test.ts | 3 + .../src/runtime/web-session-tabs-sync.ts | 166 ++++++- ...ent-hook-listener-claude-subagents.test.ts | 68 ++- src/shared/agent-hook-listener.ts | 60 ++- .../agent-status-field-normalization.ts | 8 + src/shared/agent-status-types.test.ts | 36 ++ src/shared/agent-status-types.ts | 9 +- src/shared/runtime-types.ts | 2 + 35 files changed, 2720 insertions(+), 86 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts create mode 100644 src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts create mode 100644 src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts create mode 100644 src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts create mode 100644 src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts create mode 100644 src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts create mode 100644 src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts diff --git a/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts b/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts index 14235654d37..5204c6b0b0b 100644 --- a/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts +++ b/src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts @@ -43,6 +43,22 @@ describe('createHookStatusSessionTabsInvalidator', () => { expect(changed(working({}, payload))).toBe(true) }) + it('invalidates when the completion stamp is added, changed, or removed', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working()) + + expect(changed(working({}, { turnCompletedAt: 100 }))).toBe(true) + expect(changed(working({}, { turnCompletedAt: 200 }))).toBe(true) + expect(changed(working())).toBe(true) + }) + + it('invalidates when the assistant body changes', () => { + const changed = createHookStatusSessionTabsInvalidator() + changed(working({}, { lastAssistantMessage: 'First answer' })) + + expect(changed(working({}, { lastAssistantMessage: 'Corrected answer' }))).toBe(true) + }) + it('ignores resume-identity rows, which the provider-session path owns', () => { const changed = createHookStatusSessionTabsInvalidator() diff --git a/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts b/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts index 59b259fa849..3aa90b530f4 100644 --- a/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts +++ b/src/main/agent-hooks/hook-status-session-tabs-invalidation.ts @@ -28,7 +28,9 @@ export function createHookStatusSessionTabsInvalidator(): { (previous.agentType ?? null) !== (next.agentType ?? null) || (previous.toolName ?? null) !== (next.toolName ?? null) || (previous.interactivePrompt ?? null) !== (next.interactivePrompt ?? null) || - (previous.interrupted ?? false) !== (next.interrupted ?? false) + (previous.interrupted ?? false) !== (next.interrupted ?? false) || + (previous.turnCompletedAt ?? null) !== (next.turnCompletedAt ?? null) || + (previous.lastAssistantMessage ?? null) !== (next.lastAssistantMessage ?? null) ) } // Why: a cleared pane must re-arm, else the memo swallows the first event of the diff --git a/src/main/agent-hooks/server-ingest-terminal-status.test.ts b/src/main/agent-hooks/server-ingest-terminal-status.test.ts index b6182529628..8661701e1dc 100644 --- a/src/main/agent-hooks/server-ingest-terminal-status.test.ts +++ b/src/main/agent-hooks/server-ingest-terminal-status.test.ts @@ -27,6 +27,97 @@ afterEach(() => { }) describe('AgentHookServer ingestTerminalStatus', () => { + it('preserves a hook turn stamp when an OSC repaint omits hook-only completion text', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setListener(listener) + + server.ingestRemote( + { + paneKey: PANE, + payload: { state: 'working', prompt: 'review the PR', agentType: 'claude' } + }, + 'conn-1' + ) + listener.mockClear() + server.ingestRemote( + { + paneKey: PANE, + payload: { + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Review complete.', + turnCompletedAt: 1_700_000_005_000 + } + }, + 'conn-1' + ) + server.ingestTerminalStatus({ + paneKey: PANE, + connectionId: 'conn-1', + payload: { state: 'working', prompt: 'review the PR', agentType: 'claude' } + }) + + expect(listener).toHaveBeenCalledOnce() + expect(server.getStatusSnapshot()[0]).toMatchObject({ + state: 'working', + lastAssistantMessage: 'Review complete.', + turnCompletedAt: 1_700_000_005_000 + }) + + server.ingestTerminalStatus({ + paneKey: PANE, + connectionId: 'conn-1', + payload: { state: 'done', prompt: 'review the PR', agentType: 'claude' } + }) + + expect(listener).toHaveBeenCalledTimes(2) + expect(listener).toHaveBeenLastCalledWith( + expect.objectContaining({ payload: expect.objectContaining({ state: 'done' }) }) + ) + }) + + it('accepts an identical-prompt hook boundary after a stamped turn', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setListener(listener) + + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'Stop', + payload: { + state: 'working', + prompt: 'check status', + agentType: 'claude', + turnCompletedAt: 1_700_000_005_000 + } + }, + 'conn-1' + ) + server.ingestRemote( + { + paneKey: PANE, + hookEventName: 'UserPromptSubmit', + payload: { state: 'working', prompt: 'check status', agentType: 'claude' } + }, + 'conn-1' + ) + + expect(listener).toHaveBeenCalledTimes(2) + expect(listener).toHaveBeenLastCalledWith( + expect.objectContaining({ + payload: expect.objectContaining({ + state: 'working', + prompt: 'check status', + turnCompletedAt: undefined + }) + }) + ) + expect(server.getStatusSnapshot()[0]?.turnCompletedAt).toBeUndefined() + }) + // Why: the OSC 9999 payload cannot carry a provider session, so letting it overwrite the row // erased the session id from persisted rows and from headless `orca serve` — which serves these // rows straight to mobile — leaving Chat UI with no transcript to subscribe to (#10630). diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index f819414ad4b..48ff387fad4 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -416,7 +416,8 @@ function toAgentStatusIpcPayload(entry: EnrichedAgentHookEventPayload): AgentSta // Why: OSC never carries model/children; omit both so an equivalent OSC ping preserves the hook-cached identity graph. function equivalentParsedAgentStatusPayload( a: ParsedAgentStatusPayload, - b: ParsedAgentStatusPayload + b: ParsedAgentStatusPayload, + preserveActiveTurnStamp = false ): boolean { return ( a.state === b.state && @@ -425,11 +426,14 @@ function equivalentParsedAgentStatusPayload( a.toolName === b.toolName && a.toolInput === b.toolInput && a.interactivePrompt === b.interactivePrompt && - a.lastAssistantMessage === b.lastAssistantMessage && + (a.lastAssistantMessage === b.lastAssistantMessage || + (preserveActiveTurnStamp && b.lastAssistantMessage === undefined)) && a.interrupted === b.interrupted && // Why: a session-boundary done must never be deduped against a cached real done — // the flag has to reach receivers deterministically (STA-3386). - a.sessionBoundary === b.sessionBoundary + a.sessionBoundary === b.sessionBoundary && + (a.turnCompletedAt === b.turnCompletedAt || + (preserveActiveTurnStamp && b.turnCompletedAt === undefined)) ) } @@ -630,6 +634,7 @@ export class AgentHookServer { private assistantMessageRetryTimers = new Map>() private codexSubagentPollTimers = new Map>() private promptSentDedupeByPaneKey = new Map() + private activeHookTurnCompletedAtByPaneKey = new Map() private promptSentHashSalt = randomBytes(16).toString('hex') private closedAgentStatusTabIds = new Set() private closedAgentStatusPaneKeys = new Set() @@ -917,6 +922,9 @@ export class AgentHookServer { prompt: payload.prompt, agentType: payload.agentType, ...(restored.state === 'done' && restored.interrupted ? { interrupted: true } : {}), + ...(restored.turnCompletedAt !== undefined + ? { turnCompletedAt: restored.turnCompletedAt } + : {}), ...(payload.subagents ? { subagents: payload.subagents } : {}) } }) @@ -1138,6 +1146,10 @@ export class AgentHookServer { payload: AgentHookEventPayload, onAccepted?: () => void ): EnrichedAgentHookEventPayload { + if (payload.hookEventName === 'UserPromptSubmit') { + // Why: the prompt boundary is authoritative even when text is unchanged; its next OSC working row must not inherit the prior cron/background turn stamp. + this.activeHookTurnCompletedAtByPaneKey.delete(payload.paneKey) + } const previous = this.state.lastStatusByPaneKey.get(payload.paneKey) as | EnrichedAgentHookEventPayload | undefined @@ -1277,6 +1289,15 @@ export class AgentHookServer { } const cachedPayload = resolveCachedClaudeCompactOwnership(previous, effectivePayload) const enriched = this.attachStatusTiming(cachedPayload, now) + if ( + typeof enriched.payload.turnCompletedAt === 'number' && + Number.isFinite(enriched.payload.turnCompletedAt) + ) { + this.activeHookTurnCompletedAtByPaneKey.set( + enriched.paneKey, + enriched.payload.turnCompletedAt + ) + } // Why: an identity-matched event can still leave the aggregate backed only by another restored child; keep liveness reconciliation eligible. if (enriched.restoredUnconfirmed) { this.runtimeObservedStatusPaneKeys.delete(enriched.paneKey) @@ -1591,6 +1612,11 @@ export class AgentHookServer { if (this.runtimeObservedStatusPaneKeys.delete(previousOwnerPaneKey)) { this.runtimeObservedStatusPaneKeys.add(toPaneKey) } + const activeTurnCompletedAt = this.activeHookTurnCompletedAtByPaneKey.get(previousOwnerPaneKey) + if (activeTurnCompletedAt !== undefined) { + this.activeHookTurnCompletedAtByPaneKey.delete(previousOwnerPaneKey) + this.activeHookTurnCompletedAtByPaneKey.set(toPaneKey, activeTurnCompletedAt) + } const authorityObservation = this.currentAuthorityObservations.get(previousOwnerPaneKey) if (authorityObservation) { const owner = parsePaneKey(toPaneKey) @@ -1646,6 +1672,7 @@ export class AgentHookServer { this.clearAssistantMessageRetry(key) this.clearCodexSubagentPoll(key) clearPaneCacheState(this.state, key) + this.activeHookTurnCompletedAtByPaneKey.delete(key) this.runtimeObservedStatusPaneKeys.delete(key) this.currentAuthorityObservations.delete(key) this.promptSentDedupeByPaneKey.delete(key) @@ -1679,6 +1706,7 @@ export class AgentHookServer { } this.legacyPaneKeyAliases.delete(legacyPaneKey) clearPaneCacheState(this.state, legacyPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(legacyPaneKey) this.currentAuthorityObservations.delete(legacyPaneKey) this.promptSentDedupeByPaneKey.delete(legacyPaneKey) if (shouldClearStablePaneKey && this.state.lastStatusByPaneKey.has(entry.stablePaneKey)) { @@ -1688,6 +1716,7 @@ export class AgentHookServer { if (shouldClearStablePaneKey) { // Why: hydrated rows live under the stable key; if this PTY dies before ptyPaneKey rebuilds, alias cleanup is the only evictor. clearPaneCacheState(this.state, entry.stablePaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(entry.stablePaneKey) this.runtimeObservedStatusPaneKeys.delete(entry.stablePaneKey) this.currentAuthorityObservations.delete(entry.stablePaneKey) this.promptSentDedupeByPaneKey.delete(entry.stablePaneKey) @@ -1829,12 +1858,15 @@ export class AgentHookServer { const previous = this.state.lastStatusByPaneKey.get(paneKey) as | EnrichedAgentHookEventPayload | undefined + const preserveActiveTurnStamp = + previous?.payload.turnCompletedAt !== undefined && + previous.payload.turnCompletedAt === this.activeHookTurnCompletedAtByPaneKey.get(paneKey) if ( !previous?.restoredUnconfirmed && previous?.connectionId === connectionId && previous.tabId === tabId && previous.worktreeId === worktreeId && - equivalentParsedAgentStatusPayload(previous.payload, event.payload) + equivalentParsedAgentStatusPayload(previous.payload, event.payload, preserveActiveTurnStamp) ) { return } @@ -2331,6 +2363,7 @@ export class AgentHookServer { return null } this.state.lastStatusByPaneKey.delete(resolvedPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(resolvedPaneKey) if (!options?.preserveAuthority) { this.hydratedLaunchTokenHashByPaneKey.delete(resolvedPaneKey) this.persistedAuthorityCommitmentsByPaneKey.delete(resolvedPaneKey) @@ -2411,6 +2444,7 @@ export class AgentHookServer { this.clearAssistantMessageRetry(paneKey) this.clearCodexSubagentPoll(paneKey) clearPaneCacheState(this.state, paneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(paneKey) this.runtimeObservedStatusPaneKeys.delete(paneKey) this.currentAuthorityObservations.delete(paneKey) this.promptSentDedupeByPaneKey.delete(paneKey) @@ -2432,6 +2466,7 @@ export class AgentHookServer { this.clearAssistantMessageRetry(resolvedPaneKey) this.clearCodexSubagentPoll(resolvedPaneKey) clearPaneCacheState(this.state, resolvedPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(resolvedPaneKey) this.currentAuthorityObservations.delete(resolvedPaneKey) this.promptSentDedupeByPaneKey.delete(resolvedPaneKey) let clearedAlias = false @@ -2441,6 +2476,7 @@ export class AgentHookServer { paneKeys.add(legacyPaneKey) paneKeys.add(stablePaneKey.stablePaneKey) clearPaneCacheState(this.state, legacyPaneKey) + this.activeHookTurnCompletedAtByPaneKey.delete(legacyPaneKey) this.currentAuthorityObservations.delete(legacyPaneKey) this.promptSentDedupeByPaneKey.delete(legacyPaneKey) clearedAlias = true diff --git a/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts index 7981366cadb..11b9fc97afd 100644 --- a/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts +++ b/src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts @@ -130,6 +130,23 @@ describe('headless hook agent-status projection (#11761)', () => { ) }) + it('publishes a gated turn end as event metadata, not stored agent status', async () => { + const turnCompletedAt = Date.now() + const runtime = await createRuntimeWithHookRows([ + hookRow({ + state: 'working', + interactivePrompt: undefined, + turnCompletedAt + }) + ]) + + const result = await runtime.listMobileSessionTabs(`id:${WORKTREE_ID}`) + const tab = result.tabs[0] + + expect(tab).toMatchObject({ type: 'terminal', turnCompletedAt }) + expect(tab?.type === 'terminal' && tab.agentStatus).not.toHaveProperty('turnCompletedAt') + }) + it('publishes no hook transport identity to clients', async () => { const agentStatus = await projectAgentStatus([ hookRow({ launchToken: 'lt-secret', promptInteractionKey: 'turn-1' }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 69b7c342438..179a05e2f2b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -11806,7 +11806,9 @@ export class OrcaRuntimeService { (previous.payload.agentType ?? null) !== (payload.agentType ?? null) || (previous.payload.toolName ?? null) !== (payload.toolName ?? null) || (previous.payload.interactivePrompt ?? null) !== (payload.interactivePrompt ?? null) || - (previous.payload.interrupted ?? false) !== (payload.interrupted ?? false) + (previous.payload.interrupted ?? false) !== (payload.interrupted ?? false) || + (previous.payload.turnCompletedAt ?? null) !== (payload.turnCompletedAt ?? null) || + (previous.payload.lastAssistantMessage ?? null) !== (payload.lastAssistantMessage ?? null) ) } @@ -32428,6 +32430,31 @@ export class OrcaRuntimeService { : livePty ? this.issuePtyHandle(livePty) : null + const projectedAgentStatus = + agentStatus ?? + this.buildPtyMobileAgentStatus( + mobileStatusPty, + tab, + terminalHandle, + retainedAgentStatus, + getHookRowsForPane + ) + const projectedStatusEntry = projectedAgentStatus.agentStatus as + | (AgentStatusEntry & { turnCompletedAt?: number }) + | undefined + const { turnCompletedAt: projectedTurnCompletedAt, ...clientStatusFields } = + projectedStatusEntry ?? {} + const clientAgentStatus = projectedStatusEntry + ? { agentStatus: clientStatusFields as AgentStatusEntry } + : {} + const rawTurnCompletedAt = + hookAgentStatus?.live?.payload.turnCompletedAt ?? + this.getHookAgentRowForPane(getHookRowsForPane(paneKey)).live?.payload.turnCompletedAt ?? + projectedTurnCompletedAt + const turnCompletedAt = + typeof rawTurnCompletedAt === 'number' && Number.isFinite(rawTurnCompletedAt) + ? rawTurnCompletedAt + : undefined tabs.push({ type: 'terminal', id: tab.id, @@ -32437,14 +32464,8 @@ export class OrcaRuntimeService { ...(tab.ptyId ? { ptyId: tab.ptyId } : {}), ...(tab.terminalTheme ? { terminalTheme: tab.terminalTheme } : {}), ...(launchAgent ? { launchAgent } : {}), - ...(agentStatus ?? - this.buildPtyMobileAgentStatus( - mobileStatusPty, - tab, - terminalHandle, - retainedAgentStatus, - getHookRowsForPane - )), + ...clientAgentStatus, + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}), ...(tab.parentLayout ? { parentLayout: tab.parentLayout } : {}), ...(tab.startupCwd ? { startupCwd: tab.startupCwd } : {}), ...(tab.color != null ? { color: tab.color } : {}), diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts index 896ecc88548..ad1aab0ddcf 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-dispose-leak.test.ts @@ -101,4 +101,76 @@ describe('agent completion coordinator identity map stays bounded (leak regressi coordinator.dispose() // isLive() still true -> remount, not teardown expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(1) }) + + it('clears per-coordinator working boundaries on reset and dispose', () => { + const live = { value: true } + const coordinator = createAgentCompletionCoordinator( + makeOptions('tab-1:leaf-working-boundary', live) + ) + const working = { + state: 'working' as const, + prompt: '', + agentType: 'claude' as const, + stateStartedAt: 1_000 + } + + coordinator.observeHookStatus(working) + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(1) + coordinator.resetCompletionState() + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(0) + + coordinator.observeHookStatus(working) + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(1) + coordinator.dispose() + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(0) + }) + + it('retains stamped replay state when a mounted sibling disposes before remount', () => { + const paneKey = 'tab-1:leaf-production-remount' + const dispatchCompletion = vi.fn() + const hookLive = { value: true } + const paneLive = { value: true } + const hookCoordinator = createAgentCompletionCoordinator({ + ...makeOptions(paneKey, hookLive), + dispatchCompletion + }) + const paneCoordinator = createAgentCompletionCoordinator({ + ...makeOptions(paneKey, paneLive), + dispatchCompletion + }) + + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_000 + }) + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_000, + turnCompletedAt: 2_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + paneLive.value = false + paneCoordinator.dispose() + const remountedLive = { value: true } + const remounted = createAgentCompletionCoordinator({ + ...makeOptions(paneKey, remountedLive), + dispatchCompletion + }) + vi.advanceTimersByTime(2_000) + remounted.observeTitleWorking() + remounted.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + hookLive.value = false + remountedLive.value = false + hookCoordinator.dispose() + remounted.dispose() + expect(getAgentCompletionCoordinatorIdentityCountForTest()).toBe(0) + }) }) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts index 130d9e98edd..f28a3579a66 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-hook-title-precedence.test.ts @@ -174,6 +174,17 @@ describe('agent completion coordinator', () => { await flushAsyncTicks() expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + result = processResult('codex') + vi.advanceTimersByTime(2_000) + await flushAsyncTicks() + result = processResult('zsh', false) + vi.advanceTimersByTime(750) + await flushAsyncTicks() + vi.advanceTimersByTime(750) + await flushAsyncTicks() + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) }) it('keeps duplicate done-only hooks inside replay guard suppressed after process inspection', async () => { diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts new file mode 100644 index 00000000000..11d3cd0aac7 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-boundary.test.ts @@ -0,0 +1,409 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentCompletionCoordinator } from './agent-completion-coordinator' +import { + HOOK_DONE_QUIET_MS, + useAgentCompletionCoordinatorLifecycle +} from './agent-completion-coordinator-test-harness' + +describe('agent completion coordinator stamped turn boundary', () => { + useAgentCompletionCoordinatorLifecycle() + + it('ignores a turn end time that cannot name a turn', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: Number.NaN + }) + + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('does not announce a gated Stop for a turn it never saw start', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('does not announce the all-clear after first seeing a stamped turn tail', () => { + const dispatchCompletion = vi.fn() + const firstCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + firstCoordinator.dispose() + + const remounted = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + remounted.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('announces a gated Stop immediately and treats the later all-clear as the same turn', () => { + const dispatchCompletion = vi.fn() + const dispatchHookLifecycle = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + dispatchHookLifecycle, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Which cells need hand-verification?', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchCompletion).toHaveBeenCalledWith( + 'claude', + expect.objectContaining({ + source: 'hook', + quietedHookDone: false, + agentStatus: expect.objectContaining({ + state: 'done', + lastAssistantMessage: 'Which cells need hand-verification?', + stateStartedAt: 1_700_000_005_000, + turnCompletedAt: 1_700_000_005_000 + }) + }) + ) + expect(dispatchHookLifecycle).toHaveBeenLastCalledWith( + expect.objectContaining({ state: 'working', turnCompletedAt: 1_700_000_005_000 }) + ) + + coordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Which cells need hand-verification?', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchHookLifecycle).toHaveBeenLastCalledWith( + expect.objectContaining({ state: 'done', turnCompletedAt: 1_700_000_005_000 }) + ) + }) + + it.each(['before', 'after'] as const)( + 'does not let a sibling OSC all-clear arriving %s the host stamp duplicate it', + (allClearOrder) => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 2_000 + }) + if (allClearOrder === 'before') { + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 5_500 + }) + } + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Review complete.', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + if (allClearOrder === 'after') { + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 5_500 + }) + } + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + } + ) + + it('does not consume a fresh turn as the all-clear for an earlier stamped tail', () => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 6_000 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 6_500 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) + + it('keeps sibling tail suppression after the host stamped all-clear arrives first', () => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + hostCoordinator.observeHookStatus({ + state: 'done', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 3_000, + turnCompletedAt: 2_500 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_500 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_500 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('releases a stamped tail when its host reports a new same-state turn', () => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + localCoordinator.observeHookStatus({ + state: 'done', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 5_500 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts new file mode 100644 index 00000000000..8e4fe3b45df --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-stamped-turn-replay.test.ts @@ -0,0 +1,388 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentCompletionCoordinator } from './agent-completion-coordinator' +import { + HOOK_DONE_QUIET_MS, + useAgentCompletionCoordinatorLifecycle +} from './agent-completion-coordinator-test-harness' + +describe('agent completion coordinator stamped turn replay', () => { + useAgentCompletionCoordinatorLifecycle() + + it.each(['pty-first', 'host-first'] as const)( + 'releases stamped fallback dedupe when the next turn arrives %s', + (workingOrder) => { + const dispatchCompletion = vi.fn() + const localCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'pty', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'hook', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const observeHostNextTurn = () => + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'second turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + if (workingOrder === 'pty-first') { + localCoordinator.observeTitleWorking() + observeHostNextTurn() + } else { + observeHostNextTurn() + localCoordinator.observeTitleWorking() + } + localCoordinator.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + } + ) + + it.each(['before', 'after'] as const)( + 'keeps sibling tail replay suppression when the pane remounts %s its all-clear', + (remountOrder) => { + const dispatchCompletion = vi.fn() + const createLocalCoordinator = () => + createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'pty', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const localCoordinator = createLocalCoordinator() + const hostCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + statusLane: 'hook', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const localAllClear = { + state: 'done' as const, + prompt: 'first turn', + agentType: 'claude' as const, + stateStartedAt: 5_500 + } + + localCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 5_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000 + }) + hostCoordinator.observeHookStatus({ + state: 'working', + prompt: 'first turn', + agentType: 'claude', + stateStartedAt: 2_000, + turnCompletedAt: 2_500 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + if (remountOrder === 'after') { + localCoordinator.observeHookStatus(localAllClear) + } + localCoordinator.dispose() + const remounted = createLocalCoordinator() + remounted.observeHookStatus(localAllClear) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + } + ) + + it('does not replay a stamped turn or its all-clear after a working title blip', () => { + const dispatchCompletion = vi.fn() + const dispatchHookLifecycle = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + dispatchHookLifecycle, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(2_000) + coordinator.observeTitleWorking() + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + coordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_050_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchHookLifecycle).toHaveBeenLastCalledWith( + expect.objectContaining({ state: 'done', turnCompletedAt: 1_700_000_005_000 }) + ) + }) + + it('does not double-fire after a live remount between the gated Stop and the all-clear', () => { + const dispatchCompletion = vi.fn() + const firstCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + firstCoordinator.dispose() + + const remounted = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + remounted.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('keeps stamped replay suppression through a working title and live remount', () => { + const dispatchCompletion = vi.fn() + const firstCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + firstCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(2_000) + firstCoordinator.observeTitleWorking() + firstCoordinator.dispose() + + const remounted = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + remounted.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('does not let a pane title duplicate another coordinator stamped completion', () => { + const dispatchCompletion = vi.fn() + const hookCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + const paneCoordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + hookCoordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(2_000) + paneCoordinator.observeTitleWorking() + paneCoordinator.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + hookCoordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + paneCoordinator.observeTitleWorking() + paneCoordinator.observeClassifiedTitleCompletion('Claude done') + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) + + it('does not let a vetoed gated Stop swallow the later all-clear', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: 'tab-1:leaf-1', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000 + }) + coordinator.observeClassifiedTitleCompletion('Claude done') + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + coordinator.observeHookStatus({ + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_000_000, + turnCompletedAt: 1_700_000_005_000 + }) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + + coordinator.observeHookStatus({ + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: 1_700_000_055_000, + turnCompletedAt: 1_700_000_005_000 + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts index a388ec17457..9614d0bbffc 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts @@ -5,6 +5,8 @@ import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-termina export type AgentCompletionStatusSnapshot = ParsedAgentStatusPayload & { stateStartedAt?: number + /** Renderer-local boundary used only to reject a delayed cross-host completion. */ + localStateStartedAt?: number } export type AgentCompletionDispatchMeta = { @@ -21,6 +23,7 @@ export type AgentAttentionDispatchMeta = { export type AgentCompletionCoordinatorOptions = { paneKey: string + statusLane?: 'hook' | 'pty' getPtyId: () => string | null getSettings: () => Pick | null | undefined inspectProcess: ( @@ -50,6 +53,7 @@ export type AgentCompletionCoordinator = { observeTitleWorking: () => void observeOutputActivity: () => void observeHookStatus: (payload: AgentCompletionStatusSnapshot) => void + seedHookStatus: (payload: AgentCompletionStatusSnapshot) => void startProcessTracking: () => void hasPendingHookDoneCompletion: () => boolean resetCompletionState: (options?: { requireFreshWorking?: boolean }) => void diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts index b3059ca6390..f0802bd123a 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator.ts @@ -31,10 +31,29 @@ type LastCompletionIdentity = { source: CompletionIdentitySource identity: string agentIdentity: string | null + /** End time of the Claude lead turn already announced while the pane stayed `working` for background inventory. The later all-clear `done` repeats this value and must not raise a second banner. */ + lastTurnCompletedAtNotified?: number } // Why: worktree switches remount a pane while the PTY/hook stream stays live, so stale completion replays must outlive one coordinator. const lastCompletionIdentityByPaneKey = new Map() +// Why: a late-pair stamped tail must suppress its replayed all-clear even though no banner was eligible. +const handledTurnCompletedAtsByPaneKey = new Map() +// Why: title fallback stays blocked only until the stamped background tail reaches its all-clear. +const pendingStampedTailByPaneKey = new Map< + string, + { + turnCompletedAt: number + originLane: string + eligibleWorkingBoundaryByLane: Map + consumedIdentityByLane: Map + tailOpen: boolean + } +>() +// Why: a sibling all-clear belongs to the stamped tail only while its exact working interval is still current. +const workingBoundaryByPaneKey = new Map>() +const coordinatorCountByPaneKey = new Map() +let nextCoordinatorId = 1 const IDLE_POLL_INTERVAL_MS = 2_000 const ACTIVE_POLL_INTERVAL_MS = 750 @@ -51,6 +70,7 @@ const COMPLETION_REPLAY_GUARD_MS = 1_000 const HOOK_DONE_QUIET_MS = 1_500 // Why: under "Approve for me" Codex resumes almost immediately, so debounce the OS attention notification so a self-resolving pause raises no false banner (#8387). const CODEX_ATTENTION_QUIET_MS = 1_500 +const HANDLED_TURN_STAMP_LIMIT = 16 const POLL_TIER_INTERVAL_MS: Record = { active: ACTIVE_POLL_INTERVAL_MS, @@ -59,6 +79,10 @@ const POLL_TIER_INTERVAL_MS: Record = { 'no-evidence': NO_EVIDENCE_POLL_INTERVAL_MS } +function isFiniteTurnCompletedAt(value: number | undefined): value is number { + return typeof value === 'number' && Number.isFinite(value) +} + function isCompletionHookState(state: ParsedAgentStatusPayload['state']): boolean { // Why: only a genuine 'done' ends a turn; 'waiting'/'blocked' go through isAttentionHookState. return state === 'done' @@ -72,6 +96,15 @@ function isAttentionHookState(state: ParsedAgentStatusPayload['state']): boolean export function createAgentCompletionCoordinator( options: AgentCompletionCoordinatorOptions ): AgentCompletionCoordinator { + const coordinatorId = nextCoordinatorId++ + const coordinatorLane = options.statusLane ?? `coordinator:${coordinatorId}` + let inheritedWorkingBoundary = pendingStampedTailByPaneKey + .get(options.paneKey) + ?.eligibleWorkingBoundaryByLane.get(coordinatorLane) + coordinatorCountByPaneKey.set( + options.paneKey, + (coordinatorCountByPaneKey.get(options.paneKey) ?? 0) + 1 + ) let disposed = false let agentIdentityEstablished = false let hasAgentRunEvidence = false @@ -169,15 +202,113 @@ export function createAgentCompletionCoordinator( return `${source}:${currentTurn}:${processSession}` } + function completionIdentityFor( + state: string, + agentType: string | undefined, + timestamp: number + ): string { + return [state, agentType ?? '', String(Math.trunc(timestamp))].join(':') + } + function hookCompletionIdentity(payload: AgentCompletionStatusSnapshot): string | null { - if (typeof payload.stateStartedAt !== 'number' || !Number.isFinite(payload.stateStartedAt)) { + // Why: `stateStartedAt` is pinned while the reported state does not change. A Claude pane held at `working` by background inventory would otherwise give every turn in the run the same identity. + const timestamp = isFiniteTurnCompletedAt(payload.turnCompletedAt) + ? payload.turnCompletedAt + : payload.stateStartedAt + if (typeof timestamp !== 'number' || !Number.isFinite(timestamp)) { return null } - return [ - payload.state, - payload.agentType ?? '', - String(Math.trunc(payload.stateStartedAt)) - ].join(':') + return completionIdentityFor(payload.state, payload.agentType, timestamp) + } + + function turnCompletedAtAlreadyHandled(turnCompletedAt: number): boolean { + return ( + handledTurnCompletedAtsByPaneKey.get(options.paneKey)?.includes(turnCompletedAt) === true || + lastCompletionIdentityByPaneKey.get(options.paneKey)?.lastTurnCompletedAtNotified === + turnCompletedAt + ) + } + + function rememberHandledTurnCompletedAt(turnCompletedAt: number): void { + const handled = handledTurnCompletedAtsByPaneKey.get(options.paneKey) ?? [] + if (handled.includes(turnCompletedAt)) { + return + } + handled.push(turnCompletedAt) + if (handled.length > HANDLED_TURN_STAMP_LIMIT) { + handled.shift() + } + handledTurnCompletedAtsByPaneKey.set(options.paneKey, handled) + } + + function recordWorkingBoundary(stateStartedAt: number | undefined): void { + let boundaries = workingBoundaryByPaneKey.get(options.paneKey) + if (!isFiniteTurnCompletedAt(stateStartedAt)) { + boundaries?.delete(coordinatorLane) + if (boundaries?.size === 0) { + workingBoundaryByPaneKey.delete(options.paneKey) + } + return + } + if (!boundaries) { + boundaries = new Map() + workingBoundaryByPaneKey.set(options.paneKey, boundaries) + } + inheritedWorkingBoundary = undefined + boundaries.set(coordinatorLane, stateStartedAt) + } + + function clearWorkingBoundary(): void { + inheritedWorkingBoundary = undefined + const boundaries = workingBoundaryByPaneKey.get(options.paneKey) + boundaries?.delete(coordinatorLane) + if (boundaries?.size === 0) { + workingBoundaryByPaneKey.delete(options.paneKey) + } + } + + function consumePendingStampedTailForAgent( + agentIdentity: string | null, + completionIdentity: string | null + ): boolean { + const pendingStampedTail = pendingStampedTailByPaneKey.get(options.paneKey) + const stampedCompletion = lastCompletionIdentityByPaneKey.get(options.paneKey) + const currentWorkingBoundary = workingBoundaryByPaneKey + .get(options.paneKey) + ?.get(coordinatorLane) + const eligibleWorkingBoundary = + pendingStampedTail?.eligibleWorkingBoundaryByLane.get(coordinatorLane) + if ( + pendingStampedTail === undefined || + stampedCompletion?.lastTurnCompletedAtNotified !== pendingStampedTail.turnCompletedAt || + agentIdentity === null || + stampedCompletion.agentIdentity !== agentIdentity || + eligibleWorkingBoundary === undefined || + (currentWorkingBoundary !== eligibleWorkingBoundary && + inheritedWorkingBoundary !== eligibleWorkingBoundary) + ) { + return false + } + pendingStampedTail.eligibleWorkingBoundaryByLane.delete(coordinatorLane) + inheritedWorkingBoundary = undefined + if (completionIdentity) { + pendingStampedTail.consumedIdentityByLane.set(coordinatorLane, completionIdentity) + } + pendingStampedTail.tailOpen = pendingStampedTail.eligibleWorkingBoundaryByLane.size > 0 + return true + } + + function consumeStampedTailForCurrentCoordinator(turnCompletedAt: number): void { + const pendingStampedTail = pendingStampedTailByPaneKey.get(options.paneKey) + if (pendingStampedTail?.turnCompletedAt !== turnCompletedAt) { + return + } + pendingStampedTail.eligibleWorkingBoundaryByLane.delete(coordinatorLane) + pendingStampedTail.tailOpen = pendingStampedTail.eligibleWorkingBoundaryByLane.size > 0 + } + + function hasUnconsumedStampedTail(): boolean { + return pendingStampedTailByPaneKey.get(options.paneKey)?.tailOpen === true } function hookCompletionAgentIdentity(payload: AgentCompletionStatusSnapshot): string | null { @@ -247,12 +378,27 @@ export function createAgentCompletionCoordinator( if (!completionIdentity) { return false } + if ( + completionIdentity.source === 'hook' && + pendingStampedTailByPaneKey + .get(options.paneKey) + ?.consumedIdentityByLane.get(coordinatorLane) === completionIdentity.identity + ) { + return true + } const previous = lastCompletionIdentityByPaneKey.get(options.paneKey) if (!previous) { return false } if (previous.source === completionIdentity.source) { - return previous.identity === completionIdentity.identity + return ( + previous.identity === completionIdentity.identity || + (completionIdentity.source === 'hook' && + consumePendingStampedTailForAgent( + completionIdentity.agentIdentity, + completionIdentity.identity + )) + ) } return ( previous.agentIdentity !== null && @@ -269,24 +415,26 @@ export function createAgentCompletionCoordinator( terminalIdleConfirmed?: boolean agentStatus?: AgentCompletionStatusSnapshot completionIdentity?: LastCompletionIdentity | null + /** Announce only. The pane is still genuinely `working` (Claude background inventory), so the synthetic `done` must not run pane lifecycle. */ + notifyWithoutLifecycle?: boolean } = {} - ): void { + ): boolean { if (source !== 'hook' && pendingHookDoneTimer !== null) { - return + return false } if (requiresFreshWorking || lastCompletedTurn === currentTurn) { - return + return false } if (!options.isLive() || !hasAgentRunEvidence) { - return + return false } const now = Date.now() const token = completionToken(source) if (token === lastCompletionToken && now - lastCompletionAt < COMPLETION_REPLAY_GUARD_MS) { - return + return false } if (completionIdentityAlreadyNotified(optionsOverride.completionIdentity)) { - return + return false } lastCompletionToken = token lastCompletionAt = now @@ -297,8 +445,19 @@ export function createAgentCompletionCoordinator( clearPendingCodexAttention() if (optionsOverride.completionIdentity) { lastCompletionIdentityByPaneKey.set(options.paneKey, optionsOverride.completionIdentity) + if (optionsOverride.completionIdentity.lastTurnCompletedAtNotified !== undefined) { + rememberHandledTurnCompletedAt( + optionsOverride.completionIdentity.lastTurnCompletedAtNotified + ) + } else { + pendingStampedTailByPaneKey.delete(options.paneKey) + } } - if (source === 'hook' && optionsOverride.agentStatus) { + if ( + source === 'hook' && + optionsOverride.agentStatus && + optionsOverride.notifyWithoutLifecycle !== true + ) { options.dispatchHookLifecycle?.(optionsOverride.agentStatus) } if (optionsOverride.quietedHookDone === true || source === 'process-exit') { @@ -309,9 +468,17 @@ export function createAgentCompletionCoordinator( ...(optionsOverride.terminalIdleConfirmed === true ? { terminalIdleConfirmed: true } : {}), ...(optionsOverride.agentStatus ? { agentStatus: optionsOverride.agentStatus } : {}) }) + } else if (optionsOverride.notifyWithoutLifecycle === true && optionsOverride.agentStatus) { + // Why: the pane is still `working`; the synthetic done must carry its own snapshot or the notification would read the pinned working row. + options.dispatchCompletion(title, { + source, + quietedHookDone: false, + agentStatus: optionsOverride.agentStatus + }) } else { options.dispatchCompletion(title) } + return true } function dispatchAttentionNotification(payload: AgentCompletionStatusSnapshot): void { @@ -449,6 +616,17 @@ export function createAgentCompletionCoordinator( function handleRecognizedProcess(process: RecognizedAgentProcess): void { pendingProcessExitAgent = null + const replayIdentity = lastCompletionIdentityByPaneKey.get(options.paneKey) + if ( + !lastForegroundAgent && + processSession > 0 && + !hasUnconsumedStampedTail() && + replayIdentity?.source === 'hook' && + replayIdentity.agentIdentity === process.agent + ) { + // Why: the prior observed process exited and this is a new same-agent process session, so its fallback completion cannot replay the old hook turn. + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } if (lastForegroundAgent?.agent !== process.agent) { if (lastForegroundAgent && hasAgentRunEvidence) { if ( @@ -509,7 +687,8 @@ export function createAgentCompletionCoordinator( const exited = lastForegroundAgent pendingProcessExitAgent = null if (options.shouldSuppressConfirmedProcessExitCompletion?.(exited) !== true) { - dispatchCompletion('process-exit', exited.processName, { + const replayIdentityBeforeExit = lastCompletionIdentityByPaneKey.get(options.paneKey) + const committed = dispatchCompletion('process-exit', exited.processName, { terminalIdleConfirmed: true, completionIdentity: { source: 'process-exit', @@ -517,6 +696,15 @@ export function createAgentCompletionCoordinator( agentIdentity: exited.agent } }) + if ( + !committed && + !hasUnconsumedStampedTail() && + replayIdentityBeforeExit?.source === 'hook' && + replayIdentityBeforeExit.agentIdentity === exited.agent + ) { + // Why: this confirmed exit consumed the hook turn's process backstop; a later process instance must be allowed to complete independently. + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } } lastForegroundAgent = null clearAgentRunEvidence() @@ -697,7 +885,9 @@ export function createAgentCompletionCoordinator( clearPendingCodexAttention() workingStatusObserved = true requiresFreshWorking = false - lastCompletionIdentityByPaneKey.delete(options.paneKey) + if (!hasUnconsumedStampedTail()) { + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } currentTurn += 1 dropPendingTitle() return true @@ -792,6 +982,67 @@ export function createAgentCompletionCoordinator( establishAgentEvidence() } if (payload.state === 'working') { + const turnCompletedAt = isFiniteTurnCompletedAt(payload.turnCompletedAt) + ? payload.turnCompletedAt + : undefined + if (turnCompletedAt !== undefined) { + const alreadyHandled = turnCompletedAtAlreadyHandled(turnCompletedAt) + if (!alreadyHandled) { + const eligibleWorkingBoundaryByLane = new Map( + workingBoundaryByPaneKey.get(options.paneKey) + ) + eligibleWorkingBoundaryByLane.delete(coordinatorLane) + pendingStampedTailByPaneKey.set(options.paneKey, { + turnCompletedAt, + originLane: coordinatorLane, + eligibleWorkingBoundaryByLane, + consumedIdentityByLane: new Map(), + tailOpen: true + }) + } + // Why: Claude's lead Stop already ended the turn; the pane stays `working` only for background inventory. Announce now, keep lifecycle on the reported working row. + if ( + workingStatusObserved && + !alreadyHandled && + lastCompletionIdentity?.lastTurnCompletedAtNotified !== turnCompletedAt + ) { + const completionSnapshot: AgentCompletionStatusSnapshot = { + ...payload, + state: 'done', + stateStartedAt: turnCompletedAt, + turnCompletedAt + } + const completionIdentity: LastCompletionIdentity = { + source: 'hook', + identity: completionIdentityFor('done', payload.agentType, turnCompletedAt), + agentIdentity: hookCompletionAgentIdentity(payload), + lastTurnCompletedAtNotified: turnCompletedAt + } + const committed = dispatchCompletion('hook', payload.agentType ?? options.paneKey, { + notifyWithoutLifecycle: true, + agentStatus: completionSnapshot, + completionIdentity + }) + if (committed) { + lastCompletionIdentity = completionIdentity + } + } else if (!workingStatusObserved) { + rememberHandledTurnCompletedAt(turnCompletedAt) + } + options.dispatchHookLifecycle?.(payload) + return + } + const pendingStampedTail = pendingStampedTailByPaneKey.get(options.paneKey) + if (pendingStampedTail?.originLane === coordinatorLane) { + pendingStampedTailByPaneKey.delete(options.paneKey) + if ( + lastCompletionIdentityByPaneKey.get(options.paneKey)?.lastTurnCompletedAtNotified === + pendingStampedTail.turnCompletedAt + ) { + lastCompletionIdentityByPaneKey.delete(options.paneKey) + } + } + recordWorkingBoundary(payload.stateStartedAt) clearPendingHookDone() // Why: resumed work cancels the debounced attention so a self-resolving pause never notifies. clearPendingCodexAttention() @@ -824,6 +1075,35 @@ export function createAgentCompletionCoordinator( establishAgentEvidence() } const hookIdentity = hookCompletionIdentity(payload) + const turnCompletedAt = isFiniteTurnCompletedAt(payload.turnCompletedAt) + ? payload.turnCompletedAt + : undefined + if ( + turnCompletedAt === undefined && + consumePendingStampedTailForAgent(hookCompletionAgentIdentity(payload), hookIdentity) + ) { + // Why: paired host hooks carry the stamp while the sibling OSC coordinator sees only the matching all-clear. + lastCompletionIdentity = hookIdentity + ? { + source: 'hook', + identity: hookIdentity, + agentIdentity: hookCompletionAgentIdentity(payload) + } + : null + options.dispatchHookLifecycle?.(payload) + return + } + if ( + turnCompletedAt !== undefined && + (turnCompletedAtAlreadyHandled(turnCompletedAt) || + lastCompletionIdentity?.lastTurnCompletedAtNotified === turnCompletedAt || + lastCompletedTurn === currentTurn) + ) { + // Why: this `done` is the background all-clear of a turn already announced (or completed from another source). Keep pane lifecycle; do not raise a second banner. + consumeStampedTailForCurrentCoordinator(turnCompletedAt) + options.dispatchHookLifecycle?.(payload) + return + } if ( hookIdentity && lastCompletionIdentity?.source === 'hook' && @@ -869,6 +1149,14 @@ export function createAgentCompletionCoordinator( } } + function seedHookStatus(payload: AgentCompletionStatusSnapshot): void { + const { turnCompletedAt, ...unstampedPayload } = payload + if (isFiniteTurnCompletedAt(turnCompletedAt)) { + rememberHandledTurnCompletedAt(turnCompletedAt) + } + observeHookStatus(unstampedPayload) + } + function markTitleCompletionNotified(title: string): void { lastCompletionIdentity = { source: 'title', @@ -903,17 +1191,31 @@ export function createAgentCompletionCoordinator( lastForegroundAgent = null requiresFreshWorking = options.requireFreshWorking ?? false inspectionGeneration += 1 + clearWorkingBoundary() } function dispose(): void { + if (disposed) { + return + } disposed = true clearPollTimer() clearPendingHookDone() clearPendingCodexAttention() dropPendingTitle() - // Why: module-scoped identity survives a live-stream remount; evict only on genuine teardown (isLive() false) so it can't leak per closed pane. - if (!options.isLive()) { + clearWorkingBoundary() + const remainingCoordinators = (coordinatorCountByPaneKey.get(options.paneKey) ?? 1) - 1 + if (remainingCoordinators > 0) { + coordinatorCountByPaneKey.set(options.paneKey, remainingCoordinators) + } else { + coordinatorCountByPaneKey.delete(options.paneKey) + } + // Why: one pane can have hook and mounted coordinators; a remount disposal must not erase replay state still owned by its live sibling. + if (remainingCoordinators <= 0 && !options.isLive()) { lastCompletionIdentityByPaneKey.delete(options.paneKey) + handledTurnCompletedAtsByPaneKey.delete(options.paneKey) + pendingStampedTailByPaneKey.delete(options.paneKey) + workingBoundaryByPaneKey.delete(options.paneKey) } } @@ -923,6 +1225,7 @@ export function createAgentCompletionCoordinator( observeTitleWorking, observeOutputActivity, observeHookStatus, + seedHookStatus, startProcessTracking, hasPendingHookDoneCompletion, resetCompletionState, @@ -932,8 +1235,18 @@ export function createAgentCompletionCoordinator( export function resetAgentCompletionCoordinatorIdentitiesForTest(): void { lastCompletionIdentityByPaneKey.clear() + handledTurnCompletedAtsByPaneKey.clear() + pendingStampedTailByPaneKey.clear() + workingBoundaryByPaneKey.clear() + coordinatorCountByPaneKey.clear() + nextCoordinatorId = 1 } export function getAgentCompletionCoordinatorIdentityCountForTest(): number { - return lastCompletionIdentityByPaneKey.size + return new Set([ + ...lastCompletionIdentityByPaneKey.keys(), + ...handledTurnCompletedAtsByPaneKey.keys(), + ...pendingStampedTailByPaneKey.keys(), + ...workingBoundaryByPaneKey.keys() + ]).size } diff --git a/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts b/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts index f93542cf196..7b973c4a0be 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-snapshot-staleness.ts @@ -8,16 +8,20 @@ export function isSupersededAgentCompletionSnapshot( if (!storedAgentStatus || !snapshot) { return false } - if (typeof snapshot.stateStartedAt !== 'number') { + const comparableStateStartedAt = snapshot.localStateStartedAt ?? snapshot.stateStartedAt + if (typeof comparableStateStartedAt !== 'number') { return storedAgentStatus.state !== snapshot.state } // Why: hook completion notifications are delayed by a quiet window; by the // time they fire, the same pane may already belong to a newer agent turn. - if (storedAgentStatus.stateStartedAt > snapshot.stateStartedAt) { + if (storedAgentStatus.stateStartedAt > comparableStateStartedAt) { return true } + const hasStampedTurn = + typeof snapshot.turnCompletedAt === 'number' && Number.isFinite(snapshot.turnCompletedAt) return ( - storedAgentStatus.stateStartedAt === snapshot.stateStartedAt && - storedAgentStatus.state !== snapshot.state + storedAgentStatus.stateStartedAt === comparableStateStartedAt && + storedAgentStatus.state !== snapshot.state && + !hasStampedTurn ) } diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 397d5bf84a7..1010bb579d8 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -2425,6 +2425,7 @@ export function connectPanePty( const agentCompletionCoordinator = createAgentCompletionCoordinator({ paneKey: cacheKey, + statusLane: 'pty', getPtyId: () => transport.getPtyId(), getSettings: () => useAppStore.getState().settings, inspectProcess: inspectRuntimeTerminalProcess, diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts index 0fa4e74dd88..1267b986827 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.test.ts @@ -190,6 +190,99 @@ describe('dispatchTerminalNotification', () => { expect(mockState.markTerminalPaneUnread).toHaveBeenCalledWith(paneKey) }) + it('builds the notification id from a completion snapshot, not the pinned working row', () => { + const pinnedWorkingStartedAt = Date.now() - 60_000 + // Why: the stored row must name the event's agent, or it is dropped for identity mismatch + // and the assertion would hold whichever side of the `??` wins. + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + stateStartedAt: pinnedWorkingStartedAt, + agentType: 'claude', + terminalTitle: 'claude' + }) + const turnCompletedAt = Date.now() + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'claude', + paneKey, + agentStatusSnapshot: { + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: turnCompletedAt + } + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ + notificationId: buildAgentNotificationId({ + worktreeId: 'wt-primary', + paneKey, + stateStartedAt: turnCompletedAt + }) + }) + ) + }) + + it.each([ + { clientStateStartedAt: 5_000, hostTurnCompletedAt: 2_000 }, + { clientStateStartedAt: 2_000, hostTurnCompletedAt: 5_000 } + ])( + 'accepts a host-stamped completion across client/host clock skew %#', + ({ clientStateStartedAt, hostTurnCompletedAt }) => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + stateStartedAt: clientStateStartedAt, + agentType: 'claude', + terminalTitle: 'claude' + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'claude', + paneKey, + agentStatusSnapshot: { + state: 'done', + prompt: 'review the PR', + agentType: 'claude', + stateStartedAt: hostTurnCompletedAt, + localStateStartedAt: clientStateStartedAt, + turnCompletedAt: hostTurnCompletedAt + } + }) + + expect(window.api.notifications.dispatch).toHaveBeenCalledWith( + expect.objectContaining({ source: 'agent-task-complete' }) + ) + } + ) + + it('drops a host-stamped completion after a newer client turn starts', () => { + mockState.agentStatusByPaneKey[paneKey] = makeAgentStatus(paneKey, { + state: 'working', + stateStartedAt: 6_000, + agentType: 'claude', + terminalTitle: 'claude' + }) + + dispatchTerminalNotification('wt-primary', { + source: 'agent-task-complete', + terminalTitle: 'claude', + paneKey, + agentStatusSnapshot: { + state: 'done', + prompt: 'previous turn', + agentType: 'claude', + stateStartedAt: 2_000, + localStateStartedAt: 5_000, + turnCompletedAt: 2_000 + } + }) + + expect(window.api.notifications.dispatch).not.toHaveBeenCalled() + }) + it('uses a live pane key when inactive worktree tab membership is not hydrated', () => { mockState.tabsByWorktree = {} diff --git a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts index 20a0ddadf89..29b4ace2c1f 100644 --- a/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/use-notification-dispatch.ts @@ -121,7 +121,7 @@ export function dispatchTerminalNotification( return } const agentNotificationStateStartedAt = - freshStoredAgentStatus?.stateStartedAt ?? eventAgentStatusSnapshot?.stateStartedAt + eventAgentStatusSnapshot?.stateStartedAt ?? freshStoredAgentStatus?.stateStartedAt // Why: main-process hook IPC can update inactive/unmounted worktrees before // the renderer's live-PTY map catches up. A fresh accepted hook snapshot is // authoritative for agent completion; title/BEL-only paths still need PTY liveness. diff --git a/src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts new file mode 100644 index 00000000000..8583c3bb0de --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-background-turn-notifications.test.ts @@ -0,0 +1,270 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ParsedAgentStatusPayload } from '../../../shared/agent-status-types' +import { createHookListenerState, normalizeHookPayload } from '../../../shared/agent-hook-listener' +import { makePaneKey } from '../../../shared/stable-pane-id' + +const dispatchTerminalNotification = vi.fn() +const dispatchAgentHookTerminalLifecycle = vi.fn() + +const HOOK_DONE_QUIET_MS = 1_500 +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = makePaneKey('tab-1', LEAF_ID) +const WORKTREE_ID = 'wt-1' + +type MockStoreState = { + settings: { + experimentalTerminalAttention?: boolean + notifications: { + enabled: boolean + agentTaskComplete: boolean + } + } + ptyIdsByTabId: Record + suppressedPtyExitIds: Record + tabsByWorktree: Record + terminalLayoutsByTabId: Record + agentLaunchConfigByPaneKey: Record + agentStatusByPaneKey: Record + getAgentLaunchConfigForStatusEntry: () => undefined + getAgentLaunchConfigForStatusMetadata: () => undefined +} + +let mockStoreState: MockStoreState + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState + } +})) + +vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ + dispatchTerminalNotification +})) + +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle +})) + +type HookRow = ParsedAgentStatusPayload & { stateStartedAt: number } + +function pinStateStartedAt( + previous: HookRow | undefined, + payload: ParsedAgentStatusPayload, + now: number +): HookRow { + return { + ...payload, + stateStartedAt: previous && previous.state === payload.state ? previous.stateStartedAt : now + } +} + +describe('Claude background-turn completion notifications', () => { + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + vi.setSystemTime(1_700_000_000_000) + dispatchTerminalNotification.mockClear() + dispatchAgentHookTerminalLifecycle.mockClear() + mockStoreState = { + settings: { + experimentalTerminalAttention: false, + notifications: { + enabled: true, + agentTaskComplete: true + } + }, + ptyIdsByTabId: { + 'tab-1': ['pty-1'] + }, + suppressedPtyExitIds: {}, + tabsByWorktree: { + [WORKTREE_ID]: [{ id: 'tab-1', ptyId: 'pty-1' }] + }, + terminalLayoutsByTabId: {}, + agentLaunchConfigByPaneKey: {}, + agentStatusByPaneKey: {}, + getAgentLaunchConfigForStatusEntry: () => undefined, + getAgentLaunchConfigForStatusMetadata: () => undefined + } + }) + + afterEach(() => { + vi.useRealTimers() + }) + + async function play(events: { at: number; payload: Record }[]): Promise<{ + banners: { at: number; body: string | undefined; stateStartedAt: number | undefined }[] + rows: HookRow[] + }> { + const { observeAgentHookCompletionForNotification } = + await import('./agent-hook-completion-notifications') + const listener = createHookListenerState() + const rows: HookRow[] = [] + let previous: HookRow | undefined + const banners: { + at: number + body: string | undefined + stateStartedAt: number | undefined + }[] = [] + + dispatchTerminalNotification.mockImplementation((_worktreeId, event) => { + banners.push({ + at: Date.now(), + body: event.agentStatusSnapshot?.lastAssistantMessage, + stateStartedAt: event.agentStatusSnapshot?.stateStartedAt + }) + }) + + for (const event of events) { + vi.setSystemTime(event.at) + const normalized = normalizeHookPayload( + listener, + 'claude', + { paneKey: PANE_KEY, payload: event.payload }, + 'production' + ) + if (!normalized) { + continue + } + const row = pinStateStartedAt(previous, normalized.payload, event.at) + previous = row + rows.push(row) + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: row + }) + vi.advanceTimersByTime(HOOK_DONE_QUIET_MS) + } + + return { banners, rows } + } + + it('announces a subagent turn at Stop with that turn’s text, not when the child later drains', async () => { + const { banners, rows } = await play([ + { + at: 1_700_000_000_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'review the PR' } + }, + { + at: 1_700_000_001_000, + payload: { + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + } + }, + { + at: 1_700_000_006_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Which cells need hand-verification?', + background_tasks: [{ id: 'a1', type: 'subagent', status: 'running' }] + } + }, + { + at: 1_700_000_055_000, + payload: { hook_event_name: 'SubagentStop', agent_id: 'a1' } + } + ]) + + const stop = rows.find((row) => row.lastAssistantMessage?.includes('hand-verification')) + expect(stop?.state).toBe('working') + expect(stop?.turnCompletedAt).toBe(1_700_000_006_000) + expect(banners).toHaveLength(1) + expect(banners[0]?.at).toBe(1_700_000_006_000) + expect(banners[0]?.body).toBe('Which cells need hand-verification?') + expect(banners[0]?.stateStartedAt).toBe(1_700_000_006_000) + }) + + it('notifies each background-shell turn with a distinct notification identity', async () => { + const { banners } = await play([ + { + at: 1_700_000_000_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'start the server' } + }, + { + at: 1_700_000_001_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Dev server is up.', + background_tasks: [{ id: 'shell-1', type: 'shell', status: 'running' }] + } + }, + { + at: 1_700_000_010_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'add a route' } + }, + { + at: 1_700_000_011_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Route added.', + background_tasks: [{ id: 'shell-1', type: 'shell', status: 'running' }] + } + } + ]) + + expect(banners).toHaveLength(2) + expect(banners[0]).toMatchObject({ + at: 1_700_000_001_000, + body: 'Dev server is up.', + stateStartedAt: 1_700_000_001_000 + }) + expect(banners[1]).toMatchObject({ + at: 1_700_000_011_000, + body: 'Route added.', + stateStartedAt: 1_700_000_011_000 + }) + expect(banners[0]?.stateStartedAt).not.toBe(banners[1]?.stateStartedAt) + }) + + it('notifies every turn under a persistent session cron with distinct ids', async () => { + const cron = [{ id: 'cron-1' }] + const { banners } = await play([ + { + at: 1_700_000_000_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'first' } + }, + { + at: 1_700_000_001_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'First done.', + session_crons: cron + } + }, + { + at: 1_700_000_002_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'second' } + }, + { + at: 1_700_000_003_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Second done.', + session_crons: cron + } + }, + { + at: 1_700_000_004_000, + payload: { hook_event_name: 'UserPromptSubmit', prompt: 'third' } + }, + { + at: 1_700_000_005_000, + payload: { + hook_event_name: 'Stop', + last_assistant_message: 'Third done.', + session_crons: cron + } + } + ]) + + expect(banners.map((banner) => banner.body)).toEqual([ + 'First done.', + 'Second done.', + 'Third done.' + ]) + expect(new Set(banners.map((banner) => banner.stateStartedAt)).size).toBe(3) + }) +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts b/src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts new file mode 100644 index 00000000000..006a848fdab --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-fresh-working.test.ts @@ -0,0 +1,122 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ParsedAgentStatusPayload } from '../../../shared/agent-status-types' +import { makePaneKey } from '../../../shared/stable-pane-id' + +const dispatchTerminalNotification = vi.fn() +const dispatchAgentHookTerminalLifecycle = vi.fn() + +const PANE_KEY = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') +const WORKTREE_ID = 'wt-1' + +type MockStoreState = { + settings: { + experimentalTerminalAttention: boolean + notifications: { enabled: boolean; agentTaskComplete: boolean } + } + ptyIdsByTabId: Record + suppressedPtyExitIds: Record + tabsByWorktree: Record + terminalLayoutsByTabId: Record + agentLaunchConfigByPaneKey: Record + agentStatusByPaneKey: Record + getAgentLaunchConfigForStatusEntry: () => undefined + getAgentLaunchConfigForStatusMetadata: () => undefined +} + +let mockStoreState: MockStoreState + +vi.mock('@/store', () => ({ + useAppStore: { getState: () => mockStoreState } +})) + +vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ + dispatchTerminalNotification +})) + +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle +})) + +function working(turnCompletedAt?: number): ParsedAgentStatusPayload { + return { + state: 'working', + prompt: 'implement notifications', + agentType: 'claude', + lastAssistantMessage: turnCompletedAt === undefined ? undefined : 'Turn complete.', + turnCompletedAt + } +} + +describe('agent hook completion fresh-working gate', () => { + beforeEach(() => { + vi.resetModules() + vi.useFakeTimers() + dispatchTerminalNotification.mockClear() + dispatchAgentHookTerminalLifecycle.mockClear() + mockStoreState = { + settings: { + experimentalTerminalAttention: false, + notifications: { enabled: true, agentTaskComplete: false } + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] }, + suppressedPtyExitIds: {}, + tabsByWorktree: { [WORKTREE_ID]: [{ id: 'tab-1', ptyId: 'pty-1' }] }, + terminalLayoutsByTabId: {}, + agentLaunchConfigByPaneKey: {}, + agentStatusByPaneKey: {}, + getAgentLaunchConfigForStatusEntry: () => undefined, + getAgentLaunchConfigForStatusMetadata: () => undefined + } + }) + + afterEach(() => vi.useRealTimers()) + + it('stays gated through a stamped working completion after re-enable', async () => { + const { + observeAgentHookCompletionForNotification, + syncAgentHookCompletionNotificationSettings + } = await import('./agent-hook-completion-notifications') + + syncAgentHookCompletionNotificationSettings() + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: { ...working(), stateStartedAt: 1_000 } + }) + + mockStoreState.settings.notifications.agentTaskComplete = true + syncAgentHookCompletionNotificationSettings() + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: working(1_500) + }) + + expect(dispatchTerminalNotification).not.toHaveBeenCalled() + + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: { ...working(), stateStartedAt: 2_000 } + }) + observeAgentHookCompletionForNotification({ + paneKey: PANE_KEY, + worktreeId: WORKTREE_ID, + payload: { ...working(2_500), stateStartedAt: 2_000 } + }) + + expect(dispatchTerminalNotification).toHaveBeenCalledTimes(1) + expect(dispatchTerminalNotification).toHaveBeenCalledWith( + WORKTREE_ID, + expect.objectContaining({ + source: 'agent-task-complete', + paneKey: PANE_KEY, + agentStatusSnapshot: expect.objectContaining({ + state: 'done', + turnCompletedAt: 2_500, + lastAssistantMessage: 'Turn complete.' + }) + }) + ) + }) +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts new file mode 100644 index 00000000000..390439d894c --- /dev/null +++ b/src/renderer/src/hooks/agent-hook-completion-notifications-import.test.ts @@ -0,0 +1,17 @@ +import { expect, it, vi } from 'vitest' + +vi.mock('@/store', () => ({ + useAppStore: vi.fn() +})) + +vi.mock('@/components/terminal-pane/use-notification-dispatch', () => ({ + dispatchTerminalNotification: vi.fn() +})) + +vi.mock('@/components/terminal-pane/agent-hook-terminal-lifecycle', () => ({ + dispatchAgentHookTerminalLifecycle: vi.fn() +})) + +it('does not read the Zustand store while importing completion notifications', async () => { + await expect(import('./agent-hook-completion-notifications')).resolves.toBeDefined() +}) diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts index 4803731fb70..63c33c7f26e 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.test.ts @@ -167,9 +167,7 @@ describe('agent hook completion notifications', () => { } }) - afterEach(() => { - vi.useRealTimers() - }) + afterEach(() => vi.useRealTimers()) it('requires fresh working after notifications start disabled and later re-enable', async () => { mockStoreState.settings.notifications.agentTaskComplete = false @@ -178,6 +176,7 @@ describe('agent hook completion notifications', () => { syncAgentHookCompletionNotificationSettings } = await import('./agent-hook-completion-notifications') + syncAgentHookCompletionNotificationSettings() mockStoreState.settings.notifications.agentTaskComplete = true syncAgentHookCompletionNotificationSettings() diff --git a/src/renderer/src/hooks/agent-hook-completion-notifications.ts b/src/renderer/src/hooks/agent-hook-completion-notifications.ts index 125c0994514..7d226331eed 100644 --- a/src/renderer/src/hooks/agent-hook-completion-notifications.ts +++ b/src/renderer/src/hooks/agent-hook-completion-notifications.ts @@ -11,6 +11,7 @@ import { collectLeafIdsInOrder } from '@/components/terminal-pane/layout-seriali import { createCodexAutoApprovalHookCompletionSuppressor } from '@/components/terminal-pane/codex-auto-approval-notification-suppression' import { dispatchAgentHookTerminalLifecycle } from '@/components/terminal-pane/agent-hook-terminal-lifecycle' import { + isAgentHookCompletionTrackingEnabled, shouldSyncAgentHookCompletionForStoreUpdate, type AgentHookCompletionStoreSnapshot } from './agent-hook-completion-store-sync' @@ -33,8 +34,8 @@ type PaneCoordinatorLivenessSnapshot = Pick< const coordinatorsByPaneKey = new Map() const paneKeysRequiringFreshWorking = new Set() -let wasAgentTaskCompleteTrackingEnabled = isAgentTaskCompleteTrackingEnabled() -let requireFreshWorkingForNewTrackingCoordinators = !wasAgentTaskCompleteTrackingEnabled +let wasAgentTaskCompleteTrackingEnabled: boolean | undefined +let requireFreshWorkingForNewTrackingCoordinators = false let lastPrunedLivenessSnapshot: PaneCoordinatorLivenessSnapshot | null = null function disposeCoordinatorForPaneKey(paneKey: string): void { @@ -111,9 +112,12 @@ function isAgentTaskCompleteTrackingEnabled(): boolean { return isAgentTaskCompleteNotificationEnabled() || isTerminalAttentionEnabled() } -export function syncAgentHookCompletionNotificationSettings(): boolean { - pruneClosedPaneCoordinators() - const enabled = isAgentTaskCompleteTrackingEnabled() +function syncAgentTaskCompleteTrackingEnabled(enabled: boolean): void { + if (wasAgentTaskCompleteTrackingEnabled === undefined) { + wasAgentTaskCompleteTrackingEnabled = enabled + requireFreshWorkingForNewTrackingCoordinators = !enabled + return + } if (enabled !== wasAgentTaskCompleteTrackingEnabled) { requireFreshWorkingForNewTrackingCoordinators = true for (const paneKey of coordinatorsByPaneKey.keys()) { @@ -121,6 +125,12 @@ export function syncAgentHookCompletionNotificationSettings(): boolean { } } wasAgentTaskCompleteTrackingEnabled = enabled +} + +export function syncAgentHookCompletionNotificationSettings(): boolean { + pruneClosedPaneCoordinators() + const enabled = isAgentTaskCompleteTrackingEnabled() + syncAgentTaskCompleteTrackingEnabled(enabled) return enabled } @@ -133,6 +143,9 @@ export function syncAgentHookCompletionNotificationsForStoreUpdate( if (!shouldSyncAgentHookCompletionForStoreUpdate(current, previous)) { return false } + if (wasAgentTaskCompleteTrackingEnabled === undefined) { + syncAgentTaskCompleteTrackingEnabled(isAgentHookCompletionTrackingEnabled(previous)) + } syncAgentHookCompletionNotificationSettings() return true } @@ -231,6 +244,7 @@ function paneCanReceiveHookCompletion(paneKey: string, tabIndex?: TabIndex): boo function createCoordinator(paneKey: string, worktreeId: string): AgentCompletionCoordinator { return createAgentCompletionCoordinator({ paneKey, + statusLane: 'hook', getPtyId: () => getPtyIdForPaneKey(paneKey), getSettings: () => useAppStore.getState().settings, inspectProcess: async (): Promise => ({ @@ -272,18 +286,28 @@ function createCoordinator(paneKey: string, worktreeId: string): AgentCompletion export function observeAgentHookCompletionForNotification({ paneKey, worktreeId, - payload + payload, + seedOnly }: { paneKey: string worktreeId: string payload: AgentCompletionStatusSnapshot + seedOnly?: boolean }): void { - pruneClosedPaneCoordinators() - if (!paneCanReceiveHookCompletion(paneKey)) { - return + // Why: replay seeds already passed indexed snapshot ownership; re-resolving every row makes startup batches quadratic. + if (seedOnly !== true) { + pruneClosedPaneCoordinators() + if (!paneCanReceiveHookCompletion(paneKey)) { + return + } } - const trackingEnabled = syncAgentHookCompletionNotificationSettings() + const trackingEnabled = isAgentTaskCompleteTrackingEnabled() + if (seedOnly === true) { + syncAgentTaskCompleteTrackingEnabled(trackingEnabled) + } else { + syncAgentHookCompletionNotificationSettings() + } let entry = coordinatorsByPaneKey.get(paneKey) if (!entry || entry.worktreeId !== worktreeId) { @@ -299,10 +323,14 @@ export function observeAgentHookCompletionForNotification({ } // Why: notification preferences may suppress alerts, but accepted hooks must // still release pane-owned cursor/cache effects after the quiet window. - if (payload.state === 'working' && trackingEnabled) { + if (payload.state === 'working' && payload.turnCompletedAt === undefined && trackingEnabled) { paneKeysRequiringFreshWorking.delete(paneKey) } - entry.coordinator.observeHookStatus(payload) + if (seedOnly === true) { + entry.coordinator.seedHookStatus(payload) + } else { + entry.coordinator.observeHookStatus(payload) + } } export function resetAgentHookCompletionNotificationCoordinators(): void { diff --git a/src/renderer/src/hooks/agent-hook-completion-store-sync.ts b/src/renderer/src/hooks/agent-hook-completion-store-sync.ts index e900c5b0c5d..fc47a20205a 100644 --- a/src/renderer/src/hooks/agent-hook-completion-store-sync.ts +++ b/src/renderer/src/hooks/agent-hook-completion-store-sync.ts @@ -24,7 +24,9 @@ export type AgentHookCompletionStoreSnapshot = { type TabVisit = () => void -function isTrackingEnabled(state: AgentHookCompletionStoreSnapshot): boolean { +export function isAgentHookCompletionTrackingEnabled( + state: AgentHookCompletionStoreSnapshot +): boolean { const notifications = state.settings?.notifications const notificationEnabled = notifications?.enabled !== false && notifications?.agentTaskComplete !== false @@ -80,7 +82,9 @@ function shouldSync( previous: AgentHookCompletionStoreSnapshot, visitTab?: TabVisit ): boolean { - if (isTrackingEnabled(current) !== isTrackingEnabled(previous)) { + if ( + isAgentHookCompletionTrackingEnabled(current) !== isAgentHookCompletionTrackingEnabled(previous) + ) { return true } if ( diff --git a/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts b/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts index 4a05afbdd11..609b3fb562c 100644 --- a/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts +++ b/src/renderer/src/hooks/ipc-events-agent-status-store-test-fixtures.ts @@ -31,6 +31,7 @@ export type AgentStatusSetData = { lastAssistantMessage?: string interrupted?: boolean sessionBoundary?: boolean + turnCompletedAt?: number terminalHandle?: string launchToken?: string providerSession?: { key: 'session_id'; id: string } diff --git a/src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts b/src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts new file mode 100644 index 00000000000..0a9cb1a6c30 --- /dev/null +++ b/src/renderer/src/hooks/useIpcEvents-agent-status-turn-completion.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + buildStoreState, + FUTURE_PANE_KEY, + type AgentStatusSetData, + type StoreLike +} from './ipc-events-agent-status-store-test-fixtures' +import { + buildWindowApi, + stubAuxiliaryModules, + stubReactSyncEffect +} from './ipc-events-agent-status-window-test-fixtures' + +describe('useIpcEvents agent status turn completion', () => { + beforeEach(() => { + vi.resetModules() + vi.unstubAllGlobals() + }) + + it('forwards turnCompletedAt through the agent-status IPC whitelist', async () => { + const setAgentStatus = vi.fn() + const observeAgentHookCompletionForNotification = vi.fn() + const onSetListenerRef: { current: ((data: AgentStatusSetData) => void) | null } = { + current: null + } + const storeState: StoreLike = buildStoreState({ + setAgentStatus, + workspaceSessionReady: true, + settings: { terminalFontSize: 13, notifications: { enabled: true, agentTaskComplete: true } }, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-future', ptyId: 'pty-1', worktreeId: 'wt-1', title: 'Claude' }] + } + }) + + stubReactSyncEffect() + vi.doMock('../store', () => ({ + useAppStore: { + subscribe: vi.fn(() => () => {}), + getState: () => storeState + } + })) + vi.doMock('./agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification, + resetAgentHookCompletionNotificationCoordinators: vi.fn(), + syncAgentHookCompletionNotificationSettings: vi.fn(), + syncAgentHookCompletionNotificationsForStoreUpdate: vi.fn() + })) + stubAuxiliaryModules() + vi.stubGlobal( + 'window', + buildWindowApi({ + onSet: (cb) => { + onSetListenerRef.current = cb + return () => {} + } + }) + ) + + const { useIpcEvents } = await import('./useIpcEvents') + useIpcEvents() + await Promise.resolve() + + if (typeof onSetListenerRef.current !== 'function') { + throw new Error('Expected agentStatus.onSet listener to be registered') + } + onSetListenerRef.current({ + paneKey: FUTURE_PANE_KEY, + tabId: 'tab-future', + worktreeId: 'wt-1', + state: 'working', + prompt: 'review the PR', + agentType: 'claude', + lastAssistantMessage: 'Which cells need hand-verification?', + turnCompletedAt: 1_700_000_005_000, + receivedAt: 1_700_000_005_000, + stateStartedAt: 1_700_000_000_000 + }) + + expect(observeAgentHookCompletionForNotification).toHaveBeenCalledWith( + expect.objectContaining({ + paneKey: FUTURE_PANE_KEY, + worktreeId: 'wt-1', + payload: expect.objectContaining({ + state: 'working', + agentType: 'claude', + turnCompletedAt: 1_700_000_005_000, + lastAssistantMessage: 'Which cells need hand-verification?' + }) + }) + ) + }) +}) diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 9817519819a..55d9cd062eb 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -3203,6 +3203,7 @@ export function useIpcEvents(): void { lastAssistantMessage: data.lastAssistantMessage, interrupted: data.interrupted, sessionBoundary: data.sessionBoundary, + turnCompletedAt: data.turnCompletedAt, // Why: same trap as interactivePrompt — this rebuild is a field whitelist, so subagent child rows vanish if omitted. subagents: data.subagents }) @@ -3406,7 +3407,7 @@ export function useIpcEvents(): void { : undefined } const applyPostCommitNotification = (): void => { - if (options?.replay !== true && statusWorktreeId) { + if (statusWorktreeId && (options?.replay !== true || resolvedPayload.state === 'working')) { // Why: local Codex/Claude hooks arrive via this main-process IPC path, not the PTY OSC fallback, so task-complete notifications must observe accepted hook state here too. const notificationPayload = typeof data.stateStartedAt === 'number' @@ -3415,7 +3416,8 @@ export function useIpcEvents(): void { observeAgentHookCompletionForNotification({ paneKey, worktreeId: statusWorktreeId, - payload: notificationPayload + payload: notificationPayload, + ...(options?.replay === true ? { seedOnly: true } : {}) }) } } diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index 0f4bae03f6a..f7c65b84c29 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -853,7 +853,7 @@ export async function refreshWebRuntimeSessionTabsSnapshot( // Why: eager refreshes can resolve after the user switched worktrees; update tabs without stealing focus. const patch = applyFreshWebSessionTabsSnapshot(state, snapshot, environmentId) return patch === state ? state : patch - }) + }, snapshot) } catch (error) { if (options.errorMode === 'throw') { throw error diff --git a/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts new file mode 100644 index 00000000000..124625efd17 --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-agent-completion-notifications.test.ts @@ -0,0 +1,325 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import { makePaneKey } from '../../../shared/stable-pane-id' +import { toWebTerminalSurfaceTabId } from '../../../shared/terminal-surface-id' + +const mocks = vi.hoisted(() => ({ + observeAgentHookCompletionForNotification: vi.fn() +})) + +vi.mock('@/hooks/agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: mocks.observeAgentHookCompletionForNotification +})) + +import { useAppStore } from '@/store' +import { + createAgentCompletionCoordinator, + resetAgentCompletionCoordinatorIdentitiesForTest +} from '@/components/terminal-pane/agent-completion-coordinator' +import { dispatchTerminalNotification } from '@/components/terminal-pane/use-notification-dispatch' +import { + markRendererOwnedAgentStatusWrite, + registerRendererOwnedAgentStatusPane, + resetRendererOwnedAgentStatusPanesForTests +} from '@/components/terminal-pane/renderer-owned-agent-status-registry' +import { + applyWebSessionTabsSnapshot, + applyWebSessionTabsStorePatch, + resetWebSessionTabsSnapshotFreshnessForTests +} from './web-session-tabs-sync' + +const ENVIRONMENT_ID = 'web-env-1' +const WORKTREE_ID = 'repo::/worktree' +const HOST_TAB_ID = 'host-tab-1' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const NOW = 1_700_000_000_000 +const initialState = useAppStore.getInitialState() + +function makeAgentSnapshot( + snapshotVersion: number, + updatedAt: number, + turnCompletedAt?: number, + state: 'working' | 'done' = 'working' +): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE_ID, + publicationEpoch: 'epoch-1', + snapshotVersion, + activeGroupId: 'host-group-1', + activeTabId: HOST_TAB_ID, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `${HOST_TAB_ID}::${LEAF_ID}`, + title: 'Claude working', + parentTabId: HOST_TAB_ID, + leafId: LEAF_ID, + isActive: true, + status: 'ready', + terminal: 'terminal-1', + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}), + agentStatus: { + state, + prompt: 'review the PR', + updatedAt, + stateStartedAt: NOW, + agentType: 'claude', + paneKey: makePaneKey(HOST_TAB_ID, LEAF_ID), + tabId: HOST_TAB_ID, + worktreeId: WORKTREE_ID, + stateHistory: [] + } + } + ] + } +} + +function applySnapshot(snapshot: RuntimeMobileSessionTabsResult, live: boolean): void { + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshot(state, snapshot, ENVIRONMENT_ID, NOW), + snapshot, + live + ) +} + +describe('paired session-tab agent completion notifications', () => { + beforeEach(() => { + vi.useFakeTimers() + mocks.observeAgentHookCompletionForNotification.mockReset() + resetAgentCompletionCoordinatorIdentitiesForTest() + resetWebSessionTabsSnapshotFreshnessForTests() + resetRendererOwnedAgentStatusPanesForTests() + resetAgentCompletionCoordinatorIdentitiesForTest() + useAppStore.setState(initialState, true) + }) + + afterEach(() => { + useAppStore.setState(initialState, true) + resetWebSessionTabsSnapshotFreshnessForTests() + resetRendererOwnedAgentStatusPanesForTests() + vi.unstubAllGlobals() + vi.useRealTimers() + }) + + it('forwards accepted working snapshots and live statuses', () => { + applySnapshot(makeAgentSnapshot(1, NOW), false) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(1) + + applySnapshot(makeAgentSnapshot(2, NOW + 1_000), true) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(2) + + const turnCompletedAt = NOW + 2_000 + applySnapshot(makeAgentSnapshot(3, NOW + 2_000, turnCompletedAt), true) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenLastCalledWith({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + worktreeId: WORKTREE_ID, + payload: expect.objectContaining({ + state: 'working', + stateStartedAt: NOW, + turnCompletedAt + }) + }) + + applySnapshot(makeAgentSnapshot(4, NOW + 3_000, turnCompletedAt + 1_000), false) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(4) + }) + + it('announces one live stamped completion after a late-pair working seed', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + + applySnapshot(makeAgentSnapshot(1, NOW), false) + applySnapshot(makeAgentSnapshot(2, NOW + 1_000, NOW + 1_000), true) + + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + }) + + it('announces a host-stamped turn while client OSC status owns the pane', () => { + const paneKey = makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID) + registerRendererOwnedAgentStatusPane(paneKey, ENVIRONMENT_ID) + markRendererOwnedAgentStatusWrite(paneKey) + useAppStore.setState({ + agentStatusByPaneKey: { + [paneKey]: { + state: 'working', + prompt: 'review the PR', + updatedAt: NOW, + stateStartedAt: NOW, + agentType: 'claude', + paneKey, + tabId: toWebTerminalSurfaceTabId(HOST_TAB_ID), + worktreeId: WORKTREE_ID, + stateHistory: [] + } + } + }) + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey, + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + + applySnapshot(makeAgentSnapshot(1, NOW - 2_000), true) + const turnCompletedAt = NOW + 20_000 + applySnapshot(makeAgentSnapshot(2, NOW - 1_000, turnCompletedAt), true) + applySnapshot(makeAgentSnapshot(3, NOW, turnCompletedAt, 'done'), true) + + expect(useAppStore.getState().agentStatusByPaneKey[paneKey]?.state).toBe('working') + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(3) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + payload: expect.objectContaining({ + localStateStartedAt: NOW, + turnCompletedAt + }) + }) + ) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + expect(dispatchCompletion).toHaveBeenCalledWith( + 'claude', + expect.objectContaining({ + source: 'hook', + quietedHookDone: false, + agentStatus: expect.objectContaining({ + state: 'done', + prompt: 'review the PR', + turnCompletedAt + }) + }) + ) + }) + + it('keeps a delayed host stamp bound to the prior client turn', () => { + const paneKey = makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID) + registerRendererOwnedAgentStatusPane(paneKey, ENVIRONMENT_ID) + markRendererOwnedAgentStatusWrite(paneKey) + const setClientTurn = (stateStartedAt: number, prompt: string): void => { + useAppStore.setState({ + agentStatusByPaneKey: { + [paneKey]: { + state: 'working', + prompt, + updatedAt: NOW, + stateStartedAt, + agentType: 'claude', + paneKey, + tabId: toWebTerminalSurfaceTabId(HOST_TAB_ID), + worktreeId: WORKTREE_ID, + stateHistory: [] + } + } + }) + } + + setClientTurn(5_000, 'first turn') + applySnapshot(makeAgentSnapshot(1, NOW - 2_000), true) + setClientTurn(6_000, 'second turn') + applySnapshot(makeAgentSnapshot(2, NOW - 1_500), true) + const turnCompletedAt = NOW + 20_000 + applySnapshot(makeAgentSnapshot(3, NOW - 1_000, turnCompletedAt), true) + + const delayedPayload = + mocks.observeAgentHookCompletionForNotification.mock.calls.at(-1)?.[0]?.payload + expect(delayedPayload).toMatchObject({ + localStateStartedAt: 5_000, + turnCompletedAt + }) + + const notificationDispatch = vi.fn() + vi.stubGlobal('window', { api: { notifications: { dispatch: notificationDispatch } } }) + dispatchTerminalNotification(WORKTREE_ID, { + source: 'agent-task-complete', + terminalTitle: 'Claude working', + paneKey, + agentCompletionSource: 'hook', + agentStatusSnapshot: delayedPayload + }) + + expect(notificationDispatch).not.toHaveBeenCalled() + }) + + it('suppresses a stamped reconnect tail and its live all-clear', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + + applySnapshot(makeAgentSnapshot(1, NOW, NOW), false) + applySnapshot(makeAgentSnapshot(2, NOW + 1_000, NOW, 'done'), true) + + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(2) + expect(dispatchCompletion).not.toHaveBeenCalled() + }) + + it('seeds an older restarted-host stamp without replaying its completion', () => { + const dispatchCompletion = vi.fn() + const coordinator = createAgentCompletionCoordinator({ + paneKey: makePaneKey(toWebTerminalSurfaceTabId(HOST_TAB_ID), LEAF_ID), + statusLane: 'hook', + getPtyId: () => 'pty-1', + getSettings: () => null, + inspectProcess: vi.fn(), + dispatchCompletion, + isLive: () => true + }) + mocks.observeAgentHookCompletionForNotification.mockImplementation(({ payload, seedOnly }) => + seedOnly ? coordinator.seedHookStatus(payload) : coordinator.observeHookStatus(payload) + ) + const firstTurnCompletedAt = NOW + 100_000 + const secondTurnCompletedAt = NOW + 200_000 + + applySnapshot(makeAgentSnapshot(1, NOW), false) + applySnapshot(makeAgentSnapshot(2, NOW + 1_000, firstTurnCompletedAt), true) + expect(dispatchCompletion).toHaveBeenCalledTimes(1) + applySnapshot(makeAgentSnapshot(3, NOW + 2_000), true) + expect(mocks.observeAgentHookCompletionForNotification).toHaveBeenCalledTimes(3) + applySnapshot(makeAgentSnapshot(4, NOW + 3_000, secondTurnCompletedAt), true) + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + applySnapshot(makeAgentSnapshot(5, NOW + 4_000), true) + + applySnapshot( + { + ...makeAgentSnapshot(1, NOW + 5_000, firstTurnCompletedAt), + publicationEpoch: 'epoch-2' + }, + false + ) + applySnapshot( + { + ...makeAgentSnapshot(2, NOW + 6_000, firstTurnCompletedAt, 'done'), + publicationEpoch: 'epoch-2' + }, + true + ) + vi.advanceTimersByTime(1_500) + + expect(dispatchCompletion).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync.test.ts b/src/renderer/src/runtime/web-session-tabs-sync.test.ts index 372c4eb4d5e..e6ddab72906 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.test.ts @@ -35,6 +35,9 @@ vi.mock('../store', () => ({ } })) +vi.mock('@/hooks/agent-hook-completion-notifications', () => ({ + observeAgentHookCompletionForNotification: vi.fn() +})) describe('applyWebSessionTabsSnapshot', () => { beforeEach(resetWebSessionTabsSyncTestState) diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 2a5eb362dfb..4daf9d82399 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -6,9 +6,11 @@ import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants' import { AGENT_STATUS_STALE_AFTER_MS, + pickParsedAgentStatusPayload, type AgentStatusEntry } from '../../../shared/agent-status-types' import { agentEntryCompletionAt } from '../../../shared/agent-completion-time' +import { normalizeTurnCompletedAtField } from '../../../shared/agent-status-field-normalization' import { agentProviderSessionsEqual } from '../../../shared/agent-session-resume' import type { RuntimeMobileSessionTabsResult, @@ -96,6 +98,7 @@ import { resetWebSessionBrowserPlacementsForTests } from './web-session-browser-placement' import { suppressE2eWebRuntimeBrowserSnapshot } from './web-runtime-browser-creation-e2e-fault' +import { observeAgentHookCompletionForNotification } from '@/hooks/agent-hook-completion-notifications' import { buildWebSessionExistingTabIndex, type WebSessionExistingTabIndex @@ -158,6 +161,17 @@ const hostSessionTabMappingKeysByEnvironmentAndWorktree = new Map< string, Map> >() +// Why: a delayed host stamp belongs to the client boundary seen with its preceding ordered frame. +const hostWorkingClientBoundaryByPaneKey = new Map< + string, + { + hostStateStartedAt: number + hostPrompt: string + clientStateStartedAt: number + stamped: boolean + } +>() +const HOST_WORKING_CLIENT_BOUNDARY_LIMIT = 512 let receivedSessionTabsFrameSequence = 0 type TerminalSurface = RuntimeMobileSessionTerminalClientTab @@ -644,6 +658,7 @@ export function resetWebSessionTabsSnapshotFreshnessForTests(): void { lastHostTerminalTabCountByWorktree.clear() hostSessionTabIdByLocalKey.clear() hostSessionTabMappingKeysByEnvironmentAndWorktree.clear() + hostWorkingClientBoundaryByPaneKey.clear() resetWebSessionBrowserPlacementsForTests() } @@ -3407,18 +3422,138 @@ function applyWebSessionTabsSnapshotOperations( } export function applyWebSessionTabsStorePatch( - buildPatch: (state: AppState) => WebSessionTabsSyncState | Partial + buildPatch: (state: AppState) => WebSessionTabsSyncState | Partial, + agentStatusSnapshots?: RuntimeMobileSessionTabsResult | readonly RuntimeMobileSessionTabsResult[], + allowCompletionNotification = false ): void { let mirroredAgentStatusChanged = false + const acceptedNotificationStatuses: { + paneKey: string + worktreeId: string + seedOnly?: true + payload: ReturnType & { + stateStartedAt: number + localStateStartedAt?: number + } + }[] = [] useAppStore.setState((state) => { const patch = buildPatch(state) mirroredAgentStatusChanged = patch !== state && Object.hasOwn(patch, 'agentStatusByPaneKey') + if (agentStatusSnapshots) { + const nextAgentStatuses = patch.agentStatusByPaneKey ?? state.agentStatusByPaneKey + const snapshots = Array.isArray(agentStatusSnapshots) + ? agentStatusSnapshots + : [agentStatusSnapshots] + for (const snapshot of snapshots) { + for (const surface of snapshot.tabs) { + if (surface.type !== 'terminal') { + continue + } + const remapped = remapHostAgentStatus(surface) + const accepted = remapped ? nextAgentStatuses[remapped.paneKey] : undefined + const turnCompletedAt = remapped + ? normalizeTurnCompletedAtField(surface.turnCompletedAt, remapped.state) + : undefined + if (remapped?.state === 'done' && turnCompletedAt === undefined) { + hostWorkingClientBoundaryByPaneKey.delete(remapped.paneKey) + } + // Why: client OSC owns display state, but only the host hook stream carries background-turn stamps. + const clientOwnedNotification = Boolean( + remapped && + isClientAuthoritativeAgentStatusPane(remapped.paneKey) && + (remapped.state === 'working' || turnCompletedAt !== undefined) + ) + if ( + !remapped || + (!clientOwnedNotification && + (!accepted || + accepted === state.agentStatusByPaneKey[remapped.paneKey] || + !agentStatusEntryEqual(accepted, remapped))) + ) { + continue + } + const notificationStatus = clientOwnedNotification ? remapped : accepted + if (!notificationStatus) { + continue + } + const currentClientStateStartedAt = clientOwnedNotification + ? state.agentStatusByPaneKey[notificationStatus.paneKey]?.stateStartedAt + : undefined + let localStateStartedAt = currentClientStateStartedAt + if ( + clientOwnedNotification && + notificationStatus.state === 'working' && + currentClientStateStartedAt !== undefined + ) { + if (turnCompletedAt === undefined) { + const retainedBoundary = hostWorkingClientBoundaryByPaneKey.get( + notificationStatus.paneKey + ) + if ( + !retainedBoundary || + retainedBoundary.hostStateStartedAt !== notificationStatus.stateStartedAt || + retainedBoundary.hostPrompt !== notificationStatus.prompt || + retainedBoundary.stamped + ) { + hostWorkingClientBoundaryByPaneKey.delete(notificationStatus.paneKey) + hostWorkingClientBoundaryByPaneKey.set(notificationStatus.paneKey, { + hostStateStartedAt: notificationStatus.stateStartedAt, + hostPrompt: notificationStatus.prompt, + clientStateStartedAt: currentClientStateStartedAt, + stamped: false + }) + if (hostWorkingClientBoundaryByPaneKey.size > HOST_WORKING_CLIENT_BOUNDARY_LIMIT) { + const oldestPaneKey = hostWorkingClientBoundaryByPaneKey.keys().next().value + if (oldestPaneKey !== undefined) { + hostWorkingClientBoundaryByPaneKey.delete(oldestPaneKey) + } + } + } + } else { + const retainedBoundary = hostWorkingClientBoundaryByPaneKey.get( + notificationStatus.paneKey + ) + if ( + retainedBoundary?.hostStateStartedAt === notificationStatus.stateStartedAt && + retainedBoundary.hostPrompt === notificationStatus.prompt + ) { + localStateStartedAt = retainedBoundary.clientStateStartedAt + retainedBoundary.stamped = true + } + } + } + if (!allowCompletionNotification && notificationStatus.state !== 'working') { + continue + } + acceptedNotificationStatuses.push({ + paneKey: notificationStatus.paneKey, + worktreeId: notificationStatus.worktreeId ?? snapshot.worktree, + ...(!allowCompletionNotification ? { seedOnly: true as const } : {}), + payload: { + ...pickParsedAgentStatusPayload({ + ...notificationStatus, + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}) + }), + stateStartedAt: notificationStatus.stateStartedAt, + ...(clientOwnedNotification + ? { + localStateStartedAt + } + : {}) + } + }) + } + } + } return patch }) // Why: paired-web snapshots bypass setAgentStatus, so arm the stale-boundary timer explicitly like local hook events do. if (mirroredAgentStatusChanged) { useAppStore.getState().scheduleAgentStatusFreshness() } + for (const status of acceptedNotificationStatuses) { + observeAgentHookCompletionForNotification(status) + } } function loadInitialWebSessionTabs( @@ -3485,8 +3620,9 @@ function loadInitialWebSessionTabs( receivedFrames[index]! ) ) - applyWebSessionTabsStorePatch((state) => - applyFreshWebSessionTabsSnapshots(state, applicable, environmentId) + applyWebSessionTabsStorePatch( + (state) => applyFreshWebSessionTabsSnapshots(state, applicable, environmentId), + applicable ) } finally { for (const finishRecovery of finishRecoveries) { @@ -3793,8 +3929,9 @@ export function useWebSessionTabsSync(): void { batch.deferredRepairWorktrees.delete(worktreeId) } if (operations.length > 0) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshotOperations(state, operations) + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshotOperations(state, operations), + operations.map(({ snapshot }) => snapshot) ) } finishVisibilityResumeBatchIfIdle(batch) @@ -4103,8 +4240,10 @@ export function useWebSessionTabsSync(): void { : [] ) if (freshSnapshots.length > 0) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshots(state, freshSnapshots, environmentId) + applyWebSessionTabsStorePatch( + (state) => + applyWebSessionTabsSnapshots(state, freshSnapshots, environmentId), + freshSnapshots ) } const freshSnapshotSet = new Set(freshSnapshots) @@ -4173,8 +4312,10 @@ export function useWebSessionTabsSync(): void { acceptReplayedWebSessionTabsSnapshot(environmentId, recovered.worktree) } if (shouldApplyWebSessionTabsSnapshot(recovered, environmentId)) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshot(state, recovered, environmentId) + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshot(state, recovered, environmentId), + recovered, + event.type === 'updated' && !replayed ) recordVisibilityResumeSnapshot(environmentId, recovered, receivedFrame) } @@ -4296,8 +4437,11 @@ export function useWebSessionTabsSync(): void { skipWakeRespawn: shouldSkipWebRuntimeWakeTerminalRespawn(activeWorktreeId) }) if (fresh) { - applyWebSessionTabsStorePatch((state) => - applyWebSessionTabsSnapshot(state, recovered, environmentId) + const replayed = isRuntimeSubscriptionReplayResponse(response) + applyWebSessionTabsStorePatch( + (state) => applyWebSessionTabsSnapshot(state, recovered, environmentId), + recovered, + event.type === 'updated' && !replayed ) recordVisibilityResumeSnapshotRef.current(environmentId, recovered, receivedFrame) } diff --git a/src/shared/agent-hook-listener-claude-subagents.test.ts b/src/shared/agent-hook-listener-claude-subagents.test.ts index 15f0ac57252..5278247cf8d 100644 --- a/src/shared/agent-hook-listener-claude-subagents.test.ts +++ b/src/shared/agent-hook-listener-claude-subagents.test.ts @@ -37,6 +37,64 @@ describe('shared agent-hook-listener', () => { const stop = claudeEvent({ hook_event_name: 'Stop', background_tasks: [] }) expect(stop?.payload.state).toBe('done') expect(stop?.payload.subagents).toBeUndefined() + expect(stop?.payload.turnCompletedAt).toBeUndefined() + }) + + it('stamps turnCompletedAt on a gated lead Stop and repeats it on the all-clear', () => { + vi.useFakeTimers() + try { + vi.setSystemTime(1_700_000_005_000) + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'review the PR' }) + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + }) + const stop = claudeEvent({ + hook_event_name: 'Stop', + last_assistant_message: 'Which cells need hand-verification?', + background_tasks: [ + { + id: 'a1', + type: 'subagent', + status: 'running', + description: 'Review loop', + agent_type: 'general-purpose' + } + ] + }) + expect(stop?.payload.state).toBe('working') + expect(stop?.payload.turnCompletedAt).toBe(1_700_000_005_000) + expect(stop?.payload.lastAssistantMessage).toBe('Which cells need hand-verification?') + + vi.setSystemTime(1_700_000_055_000) + const drained = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'a1' }) + expect(drained?.payload.state).toBe('done') + expect(drained?.payload.turnCompletedAt).toBe(1_700_000_005_000) + + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'next turn' }) + const nextStop = claudeEvent({ hook_event_name: 'Stop', background_tasks: [] }) + expect(nextStop?.payload.state).toBe('done') + expect(nextStop?.payload.turnCompletedAt).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) + + it('does not stamp an interrupted Stop even while a child still gates the pane', () => { + claudeEvent({ hook_event_name: 'UserPromptSubmit', prompt: 'review the PR' }) + claudeEvent({ + hook_event_name: 'SubagentStart', + agent_id: 'a1', + agent_type: 'general-purpose' + }) + const stop = claudeEvent({ + hook_event_name: 'Stop', + is_interrupt: true, + background_tasks: [{ id: 'a1', type: 'subagent', status: 'running' }] + }) + expect(stop?.payload.state).toBe('working') + expect(stop?.payload.turnCompletedAt).toBeUndefined() }) it.each([ @@ -631,8 +689,14 @@ describe('shared agent-hook-listener', () => { // Why: the lead already finished; the answer resumes the child, so the // emitted state is gated up to working only while that child still runs. - expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ state: 'working' }) - expect(state.claudeLeadStateByPaneKey.get(PANE_KEY)).toEqual({ state: 'done' }) + expect(clearClaudeAnsweredQuestionWait(state, PANE_KEY)).toEqual({ + state: 'working', + turnCompletedAt: expect.any(Number) + }) + expect(state.claudeLeadStateByPaneKey.get(PANE_KEY)).toEqual({ + state: 'done', + turnCompletedAt: expect.any(Number) + }) const drained = claudeEvent({ hook_event_name: 'SubagentStop', agent_id: 'a1' }) expect(drained?.payload.state).toBe('done') diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 82c9bfc9fec..ec9a947b94c 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -156,8 +156,10 @@ export type ClaudeLeadTurnState = { waitingAgentId?: string /** Tool call that owns the wait; late completions from parallel sibling tools must not dismiss its card. */ waitingToolUseId?: string + /** End time of the lead turn closed while background inventory kept the pane `working`. Repeated on the later all-clear `done`. */ + turnCompletedAt?: number /** Lead state a child-induced wait displaced, restored when the wait clears; can't invent 'working' since the done-gate only downgrades done→working, never back. */ - stateBeforeWait?: Pick + stateBeforeWait?: Pick } type CodexLeadTurnState = { @@ -2753,7 +2755,7 @@ function clearClaudePendingWaitForAgent( export function clearClaudeAnsweredQuestionWait( state: HookListenerState, paneKey: string -): Pick { +): Pick { const lead = state.claudeLeadStateByPaneKey.get(paneKey) const restored = lead?.state === 'waiting' @@ -2768,7 +2770,15 @@ export function clearClaudeAnsweredQuestionWait( : {} ) const effectiveState = resolveClaudePaneState(state, paneKey, restored) - return effectiveState === restored.state ? restored : { state: effectiveState } + return effectiveState === restored.state + ? restored + : { + state: effectiveState, + ...(restored.interrupted ? { interrupted: true as const } : {}), + ...(restored.turnCompletedAt !== undefined + ? { turnCompletedAt: restored.turnCompletedAt } + : {}) + } } /** Re-emit the cached lead state without touching its tool/prompt caches; child churn and parallel completions must not dismiss live cards. */ @@ -2802,7 +2812,9 @@ function buildClaudeCachedLeadStatusPayload( interrupted: lead?.interrupted }), updateToolSnapshot: false, - interrupted: lead?.interrupted + interrupted: lead?.interrupted, + // Why: draining the last background child is this turn's all-clear; the stamp lets a consumer pair it with the announcement already sent. + turnCompletedAt: lead?.turnCompletedAt }) } @@ -2969,7 +2981,8 @@ function normalizeClaudeEvent( return buildClaudeStatusPayload(state, eventName, promptText, paneKey, hookPayload, { stateName: resolveClaudePaneState(state, paneKey, restored), updateToolSnapshot: true, - interrupted: restored.interrupted + interrupted: restored.interrupted, + turnCompletedAt: restored.turnCompletedAt }) } @@ -3001,17 +3014,14 @@ function normalizeClaudeEvent( ? previousLead.stateBeforeWait : { state: previousLead.state, - ...(previousLead.interrupted ? { interrupted: true as const } : {}) + ...(previousLead.interrupted ? { interrupted: true as const } : {}), + // Why: a child's permission pause displaces an already-finished lead; keep the end time so the later drain is still that turn's tail. + ...(previousLead.turnCompletedAt !== undefined + ? { turnCompletedAt: previousLead.turnCompletedAt } + : {}) } : undefined const waitingToolUseId = eventToolUseId ?? previousLead?.waitingToolUseId - state.claudeLeadStateByPaneKey.set(paneKey, { - state: reportedStateName, - ...(interrupted ? { interrupted } : {}), - ...(isWaitingInducing && eventAgentId ? { waitingAgentId: eventAgentId } : {}), - ...(isAskUserQuestionWait && waitingToolUseId !== undefined ? { waitingToolUseId } : {}), - ...(stateBeforeWait ? { stateBeforeWait } : {}) - }) if (interrupted && eventAgentId === undefined) { state.claudeRunningNonAgentTaskPaneKeys.delete(paneKey) @@ -3022,6 +3032,25 @@ function normalizeClaudeEvent( state: reportedStateName, interrupted }) + // Why: the lead already ended — the pane stays `working` only because background inventory is still registered. `stateStartedAt` is pinned for that whole run, so this end time is the per-turn identity and the later all-clear's pair key. + const turnCompletedAt = + eventAgentId === undefined && + isTurnBoundary && + reportedStateName === 'done' && + effectiveState === 'working' && + interrupted !== true + ? Date.now() + : undefined + + state.claudeLeadStateByPaneKey.set(paneKey, { + state: reportedStateName, + ...(interrupted ? { interrupted } : {}), + ...(isWaitingInducing && eventAgentId ? { waitingAgentId: eventAgentId } : {}), + ...(isAskUserQuestionWait && waitingToolUseId !== undefined ? { waitingToolUseId } : {}), + ...(stateBeforeWait ? { stateBeforeWait } : {}), + ...(turnCompletedAt !== undefined ? { turnCompletedAt } : {}) + }) + const effectiveRoster = state.claudeSubagentRosterByPaneKey.get(paneKey) if ( isTurnBoundary && @@ -3039,7 +3068,8 @@ function normalizeClaudeEvent( return buildClaudeStatusPayload(state, eventName, promptText, paneKey, hookPayload, { stateName: effectiveState, updateToolSnapshot: true, - interrupted + interrupted, + turnCompletedAt }) } @@ -3054,6 +3084,7 @@ function buildClaudeStatusPayload( updateToolSnapshot: boolean interrupted?: boolean sessionBoundary?: boolean + turnCompletedAt?: number } ): ParsedAgentStatusPayload | null { // Why: child-driven refreshes are roster bookkeeping, not lead tool activity; read the cached snapshot without merging so they can't clear a live AskUserQuestion card or clobber the tool preview. @@ -3078,6 +3109,7 @@ function buildClaudeStatusPayload( lastAssistantMessage: snapshot.lastAssistantMessage, interrupted: options.interrupted, sessionBoundary: options.sessionBoundary, + turnCompletedAt: options.turnCompletedAt, subagents: claudeRosterToSnapshots(state.claudeSubagentRosterByPaneKey.get(paneKey)) }) } diff --git a/src/shared/agent-status-field-normalization.ts b/src/shared/agent-status-field-normalization.ts index a56d4c412b4..98b830a7375 100644 --- a/src/shared/agent-status-field-normalization.ts +++ b/src/shared/agent-status-field-normalization.ts @@ -211,3 +211,11 @@ export function normalizeOptionalMultilineField( const normalized = normalizeMultilineField(value, maxLength) return normalized.length > 0 ? normalized : undefined } + +/** Keep a lead turn's end time only on the gated `working` row and that turn's later all-clear `done`. */ +export function normalizeTurnCompletedAtField(value: unknown, state: string): number | undefined { + if (typeof value !== 'number' || !Number.isFinite(value)) { + return undefined + } + return state === 'working' || state === 'done' ? value : undefined +} diff --git a/src/shared/agent-status-types.test.ts b/src/shared/agent-status-types.test.ts index 1dea3dc70fe..f82932f3aeb 100644 --- a/src/shared/agent-status-types.test.ts +++ b/src/shared/agent-status-types.test.ts @@ -4,6 +4,7 @@ import { isFreshNonDoneAgentStatus, parseAgentStatusPayload, normalizeAgentStatusPayload, + pickParsedAgentStatusPayload, AGENT_STATUS_JSON_STRUCTURE_LIMITS, AGENT_STATUS_MAX_FIELD_LENGTH, AGENT_STATUS_MAX_SUBAGENTS, @@ -435,6 +436,41 @@ Fix dispatch fallback preview for normalized status prompts` ).toBeUndefined() }) + it('keeps turnCompletedAt on the gated working row and its all-clear done, nowhere else', () => { + for (const state of ['working', 'done'] as const) { + expect( + parseAgentStatusPayload(`{"state":"${state}","turnCompletedAt":1767225601000}`)! + .turnCompletedAt + ).toBe(1767225601000) + } + for (const state of ['blocked', 'waiting'] as const) { + expect( + parseAgentStatusPayload(`{"state":"${state}","turnCompletedAt":1767225601000}`)! + .turnCompletedAt + ).toBeUndefined() + } + for (const raw of ['"1767225601000"', 'null', 'true']) { + expect( + parseAgentStatusPayload(`{"state":"done","turnCompletedAt":${raw}}`)!.turnCompletedAt + ).toBeUndefined() + } + for (const value of [Number.NaN, Number.POSITIVE_INFINITY]) { + expect( + normalizeAgentStatusPayload({ state: 'done', turnCompletedAt: value })!.turnCompletedAt + ).toBeUndefined() + } + }) + + it('carries turnCompletedAt through the client-visible payload projection', () => { + expect( + pickParsedAgentStatusPayload({ + state: 'working', + prompt: 'run the build', + turnCompletedAt: 1767225601000 + }).turnCompletedAt + ).toBe(1767225601000) + }) + it('requires strict boolean true for interrupted (rejects truthy non-boolean)', () => { // Why: parser uses `=== true`, so truthy string/number sentinels don't count. expect( diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index 80ce1a4f823..c1e3db6e606 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -7,7 +7,8 @@ import { normalizeInteractivePromptField, normalizeOptionalField, normalizeOptionalMultilineField, - normalizePromptField + normalizePromptField, + normalizeTurnCompletedAtField } from './agent-status-field-normalization' import { assertJsonTextStructureWithinLimits } from './json-text-structure-limit' @@ -184,6 +185,10 @@ export type AgentStatusPayload = { * completions (notifications, automation runs, unread badges, finished timestamps) * must ignore it. Only meaningful on `done`. */ sessionBoundary?: boolean + /** Wall-clock ms when the lead turn ended while Claude background inventory kept the pane `working`. + * `stateStartedAt` stays pinned for that whole working run, so this is the per-turn identity. + * Present on the gated `working` row and that turn's later all-clear `done`. Event-only — not stored on AgentStatusEntry. */ + turnCompletedAt?: number /** Live in-process children of the reporting session. See AgentStatusEntry. */ subagents?: AgentSubagentSnapshot[] } @@ -217,6 +222,7 @@ export function pickParsedAgentStatusPayload( : {}), ...(row.interrupted !== undefined ? { interrupted: row.interrupted } : {}), ...(row.sessionBoundary !== undefined ? { sessionBoundary: row.sessionBoundary } : {}), + ...(row.turnCompletedAt !== undefined ? { turnCompletedAt: row.turnCompletedAt } : {}), ...(row.subagents !== undefined ? { subagents: row.subagents } : {}) } } @@ -417,6 +423,7 @@ function normalizeAgentStatusObject(parsed: unknown): ParsedAgentStatusPayload | // Why: only meaningful on `done`; coerce to undefined elsewhere so it can't leak stale truth across transitions. interrupted: obj.interrupted === true && state === 'done' ? true : undefined, sessionBoundary: obj.sessionBoundary === true && state === 'done' ? true : undefined, + turnCompletedAt: normalizeTurnCompletedAtField(obj.turnCompletedAt, state), subagents: normalizeSubagentsField(obj.subagents) } } diff --git a/src/shared/runtime-types.ts b/src/shared/runtime-types.ts index d6b257ca85f..69c8c972e36 100644 --- a/src/shared/runtime-types.ts +++ b/src/shared/runtime-types.ts @@ -181,6 +181,8 @@ export type RuntimeMobileSessionTerminalTab = { ptyId?: string | null terminalTheme?: RuntimeMobileTerminalTheme agentStatus?: AgentStatusEntry | null + /** Event-only lead-turn end time for paired clients; never persisted in AgentStatusEntry. */ + turnCompletedAt?: number launchAgent?: TuiAgent startupCwd?: string parentLayout?: TerminalLayoutSnapshot From 02ba70a847b60e59fa5cef79896415eea17a3ec0 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 16 Aug 2026 20:48:26 -0700 Subject: [PATCH 4/8] fix(agent-hooks): make the Windows managed hook survive Claude-hooks-compat consumers (#14825) * fix(agent-hooks): make the Windows managed hook survive Claude-hooks-compat consumers `~/.claude/settings.json` is not read only by Claude Code. Third-party Claude-hooks-compat layers (cursor-agent, Devin) import the same file and reimplement hook execution, so Orca's entry has to survive consumers that support strictly less than the documented schema. Three separate defects came from assuming otherwise. 1. The entry depended on `args`, which a compat consumer ignores. `args` is valid Claude Code syntax, but cursor-agent spawns `command` alone -- so `conhost.exe` ran bare, which opens an interactive console that never closes. Hook payloads were typed into those stranded shells (#14815). The entry is now one self-contained `command` string that depends on nothing optional. 2. `conhost.exe --headless` never relayed anything. It implements the ConPTY server protocol, not a generic no-window wrapper: it does not wait for the hosted process and relays neither exit code nor stdout. Measured directly -- `conhost --headless cmd /c "echo X& exit /b 42"` yields empty stdout and no exit code, while the replacement returns both and waits. So every hook was fire-and-forget, and whatever it printed was discarded. Replaced with `-WindowStyle Hidden`, which suppresses the window and keeps wait/exit-code/stdout intact. 3. The hook never wrote anything to stdout. Guards exited silently and curl's output went to nul. Claude Code documents empty stdout as "no decision", but cursor-agent treats PreToolUse as a permission gate, fails to parse empty stdout as JSON, and blocks the tool call -- so every shell command in every cursor-agent session on Windows failed (#14818). The script now writes `{}` first, on both the Windows and POSIX branches, which is documented to be identical to writing nothing for real Claude Code. Gemini and Antigravity already did this. Defects 2 and 3 are causally linked: `{}` cannot reach any consumer while conhost is swallowing stdout, so neither fix works without the other. Also fixed while establishing the contract: - The launcher's own missing-script fallback returned empty stdout, reproducing #14818 whenever `~/.orca` was cleaned or an install was half-finished. It now emits `{}` too. - PowerShell serializes progress records to stderr as CLIXML when stderr is redirected; a consumer merging stderr into stdout would see those bytes before the JSON. Every encoded payload now silences progress. - `runtime-home-hook-command.ts` built its own launcher without window suppression -- exactly the drift #14815 asks to prevent. All launcher construction now goes through `windows-powershell-hook-launcher.ts`, so the switch list cannot be present in one installer and missing in another. - Renamed `usesWindowsHeadlessHook` to `usesWindowsPowerShellLauncher`; nothing is headless anymore, and the flag selects a launcher. Testing: the new regression test asserts the effect a consumer observes -- it runs the exact `command` string from settings.json through both cmd.exe and Git Bash, across the guard-exit, reached-curl, and missing-script paths, and parses stdout. Verified it fails when `conhost --headless` is reintroduced. The previous tests all asserted installer intent, which is why they passed through all three defects. * fix(agent-hooks): close hook launcher review gaps --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- config/tsconfig.cli.json | 1 + src/main/agent-hooks/installer-utils.test.ts | 68 +++++++++++++--- src/main/agent-hooks/installer-utils.ts | 16 ++-- .../managed-hook-stdin-lifecycle.test.ts | 76 +++++++++++++++++- .../agent-hooks/runtime-home-hook-command.ts | 26 +++++-- .../windows-powershell-hook-launcher.ts | 27 +++++++ src/main/claude/hook-service.test.ts | 77 +++++++++++++------ src/main/claude/hook-service.ts | 4 + src/main/claude/hook-settings.ts | 77 +++++++++++++------ .../hook-service-managed-install.test.ts | 23 ++++-- src/main/command-code/hook-service.test.ts | 7 +- src/main/cursor/hook-service.test.ts | 2 +- src/main/droid/hook-service.test.ts | 2 +- src/main/gemini/hook-service.test.ts | 2 +- src/main/grok/hook-service.test.ts | 8 +- 15 files changed, 322 insertions(+), 94 deletions(-) create mode 100644 src/main/agent-hooks/windows-powershell-hook-launcher.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index d7e251d0d86..7ccdd5acf2d 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -14,6 +14,7 @@ "../src/main/agent-hooks/managed-agent-hook-registry.ts", "../src/main/agent-hooks/managed-hook-script-refresh.ts", "../src/main/agent-hooks/runtime-home-hook-command.ts", + "../src/main/agent-hooks/windows-powershell-hook-launcher.ts", "../src/main/amp/agent-status-plugin-source.ts", "../src/main/amp/hook-service.ts", "../src/main/amp/managed-plugin-install-status.ts", diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index 2013865aa96..f219125b78d 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -71,7 +71,10 @@ describe('readHooksJsonWithRaw', () => { for (const contents of [`\uFEFF\uFEFF${body}`, ` \uFEFF${body}`, `{\uFEFF"hooks": {}}`]) { writeFileSync(configPath, contents, 'utf-8') - expect(readHooksJsonWithRaw(configPath)).toEqual({ raw: contents, config: null }) + expect(readHooksJsonWithRaw(configPath)).toEqual({ + raw: contents, + config: null + }) } }) @@ -82,7 +85,10 @@ describe('readHooksJsonWithRaw', () => { it('keeps the raw bytes when the contents are not a JSON object', () => { writeFileSync(configPath, 'not json\n', 'utf-8') - expect(readHooksJsonWithRaw(configPath)).toEqual({ raw: 'not json\n', config: null }) + expect(readHooksJsonWithRaw(configPath)).toEqual({ + raw: 'not json\n', + config: null + }) }) }) @@ -95,7 +101,9 @@ describe('writeHooksJson', () => { writeHooksJson(configPath, { hooks: { Stop: [] } }) expect(lstatSync(configPath).isSymbolicLink()).toBe(true) - expect(JSON.parse(readFileSync(targetPath, 'utf-8'))).toEqual({ hooks: { Stop: [] } }) + expect(JSON.parse(readFileSync(targetPath, 'utf-8'))).toEqual({ + hooks: { Stop: [] } + }) }) it('does not replace a dangling hook config symlink', () => { @@ -182,9 +190,15 @@ describe('writeHooksJson', () => { }) it('updates the .bak file to the previous version on each write', () => { - const v1: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v1' }] }] } } - const v2: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v2' }] }] } } - const v3: HooksConfig = { hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v3' }] }] } } + const v1: HooksConfig = { + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v1' }] }] } + } + const v2: HooksConfig = { + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v2' }] }] } + } + const v3: HooksConfig = { + hooks: { Stop: [{ hooks: [{ type: 'command', command: 'v3' }] }] } + } writeHooksJson(configPath, v1) writeHooksJson(configPath, v2) @@ -310,7 +324,10 @@ describe('removeManagedCommands', () => { [ { hooks: [ - { type: 'command', command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' }, + { + type: 'command', + command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' + }, { type: 'command', command: 'echo keep me' } ] } @@ -381,7 +398,14 @@ describe('hookDefinitionHasManagedCommand', () => { ).toBe(true) expect( hookDefinitionHasManagedCommand( - { hooks: [{ type: 'command', command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' }] }, + { + hooks: [ + { + type: 'command', + command: '/bin/sh "/path/agent-hooks/copilot-hook.sh"' + } + ] + }, match ) ).toBe(true) @@ -539,7 +563,7 @@ describe('wrapPosixHookCommand', () => { }) const qualifiedWindowsPowerShellCommand = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ function decodeWindowsHookCommand(command: string): string { const encodedCommand = command.match(/ -EncodedCommand (\S+)$/)?.[1] @@ -549,7 +573,8 @@ function decodeWindowsHookCommand(command: string): string { function expectedDecodedWindowsHookCommand(scriptPath: string): string { const quoted = `'${scriptPath.replaceAll("'", "''")}'` - return `if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` + // Why: PowerShell progress CLIXML corrupts consumers that merge stderr into JSON stdout. + return `$ProgressPreference='SilentlyContinue'; if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` } describe('wrapWindowsHookCommand', () => { @@ -725,11 +750,32 @@ describe('wrapRuntimeHomeHookCommand', () => { process.platform === 'win32' ? join(process.env.ProgramFiles ?? 'C:\\Program Files', 'Git', 'bin', 'bash.exe') : '/bin/sh' - const result = spawnSync(shell, ['-c', command], { input: Buffer.alloc(1_000_000, 'x') }) + const result = spawnSync(shell, ['-c', command], { + input: Buffer.alloc(1_000_000, 'x') + }) expect(result.error).toBeUndefined() expect(result.status).toBe(0) }) + + it('emits neutral JSON when a lifecycle script is missing', () => { + const shell = + process.platform === 'win32' + ? join(process.env.ProgramFiles ?? 'C:\\Program Files', 'Git', 'bin', 'bash.exe') + : '/bin/sh' + const result = spawnSync( + shell, + ['-c', wrapRuntimeHomeHookCommand('missing-orca-hook', { neutralJsonWhenMissing: true })], + { + env: { ...process.env, HOME: tmpDir.replaceAll('\\', '/') }, + input: Buffer.alloc(1_000_000, 'x') + } + ) + + expect(result.error).toBeUndefined() + expect(result.status, result.stderr.toString()).toBe(0) + expect(JSON.parse(result.stdout.toString().trim())).toEqual({}) + }) }) describe('buildWindowsAgentHookPostCommand', () => { diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 8861d41f0cc..0bab6e22c2a 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -16,6 +16,7 @@ import { grantDirAcl, isPermissionError } from '../win32-utils' import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' import { resolveHooksJsonWritePath } from './hook-config-write-path' import { writeRollingFileBackup } from '../rolling-file-backup' +import { wrapWindowsPowerShellEncodedCommand } from './windows-powershell-hook-launcher' export type HookCommandConfig = { type: 'command' @@ -120,16 +121,14 @@ export function wrapPosixHookCommand(scriptPath: string, env: Record `$env:${key} = ${quotePowerShellString(value)}; `) .join('') const command = `${envPrefix}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` - const encodedCommand = Buffer.from(command, 'utf16le').toString('base64') - return `${getWindowsPowerShellExecutablePath()} -NoProfile -ExecutionPolicy Bypass -EncodedCommand ${encodedCommand}` + return wrapWindowsPowerShellEncodedCommand(command) } export const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/ diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index f805e6c25f1..3232c702dfd 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -69,6 +69,7 @@ vi.mock('os', async (importOriginal) => { import { AntigravityHookService } from '../antigravity/hook-service' import { ClaudeHookService } from '../claude/hook-service' +import { getRemoteManagedCommand } from '../claude/hook-settings' import { CodexHookService } from '../codex/hook-service' import { CommandCodeHookService } from '../command-code/hook-service' import { CopilotHookService } from '../copilot/hook-service' @@ -155,6 +156,7 @@ const LOCAL_INSTALLERS = [ type HookRun = { exitCode: number | null stdinErrors: NodeJS.ErrnoException[] + stderr: string stdout: string } @@ -164,8 +166,9 @@ function runHookProcess( env: NodeJS.ProcessEnv ): Promise { return new Promise((resolve, reject) => { - const child = spawn(executable, args, { env, stdio: ['pipe', 'pipe', 'ignore'] }) + const child = spawn(executable, args, { env, stdio: ['pipe', 'pipe', 'pipe'] }) const stdinErrors: NodeJS.ErrnoException[] = [] + let stderr = '' let stdout = '' const timeout = setTimeout(() => { child.kill('SIGKILL') @@ -178,10 +181,13 @@ function runHookProcess( child.stdout.on('data', (chunk: Buffer) => { stdout += chunk.toString() }) + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString() + }) child.stdin.on('error', (error: NodeJS.ErrnoException) => stdinErrors.push(error)) child.on('close', (exitCode) => { clearTimeout(timeout) - resolve({ exitCode, stdinErrors, stdout }) + resolve({ exitCode, stdinErrors, stderr, stdout }) }) child.stdin.end(LARGE_PAYLOAD) }) @@ -401,9 +407,75 @@ describe('Windows managed hook stdin structure', () => { } } ) + + // Why: command-shape tests missed conhost discarding the JSON consumers observe (#14818). + it.skipIf(process.platform !== 'win32')( + 'emits parseable JSON on stdout from the registered Claude hook command, through cmd.exe and Git Bash', + async () => { + const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdout-json-')) + homedirMock.mockReturnValue(home) + try { + expect(new ClaudeHookService().install().state).toBe('installed') + const settings = JSON.parse( + readFileSync(join(home, '.claude', 'settings.json'), 'utf8') + ) as { hooks: Record } + const entry = settings.hooks.PreToolUse[0].hooks[0] + expect(entry.args).toBeUndefined() + + // Why: MSYS rewrites switches and paths, so the command must survive both shells (#14815). + const gitBash = findGitBash() + const shells = [ + { name: 'cmd.exe', executable: 'cmd.exe', args: ['/d', '/c', entry.command] }, + { name: 'Git Bash', executable: gitBash, args: ['-c', entry.command] } + ] + // Why: cover guard exit, reached curl, and the launcher's missing-script fallback. + const environments = [ + { name: 'no Orca env', env: hookEnvironment({ USERPROFILE: home }) }, + { + name: 'Orca env with dead listener', + env: hookEnvironment({ + USERPROFILE: home, + ORCA_AGENT_HOOK_PORT: '59999', + ORCA_AGENT_HOOK_TOKEN: 'token', + ORCA_PANE_KEY: 'tab:leaf' + }) + }, + { + name: 'missing managed script', + env: hookEnvironment({ USERPROFILE: join(home, 'absent') }) + } + ] + for (const shell of shells) { + for (const environment of environments) { + const label = `${shell.name} / ${environment.name}` + const result = await runHookProcess(shell.executable, shell.args, environment.env) + expect(result.exitCode, `${label} exit code`).toBe(0) + expect(result.stderr, `${label} stderr`).toBe('') + expect(() => JSON.parse(result.stdout.trim()), `${label} stdout is JSON`).not.toThrow() + expect(JSON.parse(result.stdout.trim()), `${label} stdout`).toEqual({}) + } + } + } finally { + homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) + rmSync(home, { recursive: true, force: true }) + } + }, + // Why: six shell launches can overrun the default while the suite competes for cores. + 60_000 + ) }) describe.skipIf(process.platform === 'win32')('managed hook stdin lifecycle', () => { + it('emits neutral JSON when the Claude lifecycle script is missing', async () => { + const command = getRemoteManagedCommand('/home/dev/.orca/agent-hooks/claude-hook.sh') + const result = await runPosixHook(command) + + expect(result.exitCode).toBe(0) + expect(result.stdinErrors).toHaveLength(0) + expect(result.stderr).toBe('') + expect(JSON.parse(result.stdout.trim())).toEqual({}) + }) + it('captures stdin before every possible whole-script success exit', async () => { const scripts = await generatePosixScripts() for (const [agent, script] of scripts) { diff --git a/src/main/agent-hooks/runtime-home-hook-command.ts b/src/main/agent-hooks/runtime-home-hook-command.ts index 3ae887fc054..6d594c14d19 100644 --- a/src/main/agent-hooks/runtime-home-hook-command.ts +++ b/src/main/agent-hooks/runtime-home-hook-command.ts @@ -1,22 +1,32 @@ import { POSIX_HOOK_STDIN_DRAIN_COMMAND } from './hook-stdin-contract' +import { + encodeWindowsPowerShellHookCommand, + WINDOWS_POWERSHELL_HOOK_SWITCHES +} from './windows-powershell-hook-launcher' const MANAGED_SCRIPT_BASE_NAME = /^[A-Za-z0-9_-]+$/ const WINDOWS_GIT_BASH_RUNTIME_HOME_UNSAFE = '*\\&*|*\\^*|*\\(*|*\\)*|*\\;*|*,*|*=*|*%*|*\\!*' -export function wrapRuntimeHomeHookCommand(scriptBaseName: string): string { +export function wrapRuntimeHomeHookCommand( + scriptBaseName: string, + options: { neutralJsonWhenMissing?: boolean } = {} +): string { if (!MANAGED_SCRIPT_BASE_NAME.test(scriptBaseName)) { throw new Error(`Invalid managed script base name: ${scriptBaseName}`) } const windowsScript = `"$HOME/.orca/agent-hooks/${scriptBaseName}.cmd"` const posixScript = `"$HOME/.orca/agent-hooks/${scriptBaseName}.sh"` const drain = POSIX_HOOK_STDIN_DRAIN_COMMAND + const missingScriptFallback = options.neutralJsonWhenMissing ? `${drain}; printf '{}\\n'` : drain const powershell = '"$SYSTEMROOT/System32/WindowsPowerShell/v1.0/powershell.exe"' - const powershellCommand = `$homePath = $env:HOME -replace '^/([A-Za-z])/', '$1:/'; $scriptPath = Join-Path $homePath '.orca\\agent-hooks\\${scriptBaseName}.cmd'; if (Test-Path -LiteralPath $scriptPath -PathType Leaf) { & $scriptPath; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` - const encodedCommand = Buffer.from(powershellCommand, 'utf16le').toString('base64') - const powershellInvocation = `${powershell} -NoProfile -ExecutionPolicy Bypass -EncodedCommand ${encodedCommand}` - const encodedWindowsBranch = `if [ -f ${powershell} ]; then ${powershellInvocation}; else ${drain}; fi` - const windowsBranch = `if [ -f ${windowsScript} ]; then case "$HOME" in ${WINDOWS_GIT_BASH_RUNTIME_HOME_UNSAFE}) ${encodedWindowsBranch} ;; *) ${windowsScript} ;; esac; else ${drain}; fi` - const posixBranch = `if [ -f ${posixScript} ] && [ -r ${posixScript} ] && [ -x ${posixScript} ]; then /bin/sh ${posixScript}; else ${drain}; fi` + const powershellFallback = options.neutralJsonWhenMissing ? "; Write-Output '{}'" : '' + const powershellCommand = `$homePath = $env:HOME -replace '^/([A-Za-z])/', '$1:/'; $scriptPath = Join-Path $homePath '.orca\\agent-hooks\\${scriptBaseName}.cmd'; if (Test-Path -LiteralPath $scriptPath -PathType Leaf) { & $scriptPath; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null${powershellFallback}; exit 0` + const encodedCommand = encodeWindowsPowerShellHookCommand(powershellCommand) + // Why: the Git Bash and native Windows launchers must suppress windows identically (#14815). + const powershellInvocation = `${powershell} ${WINDOWS_POWERSHELL_HOOK_SWITCHES} -EncodedCommand ${encodedCommand}` + const encodedWindowsBranch = `if [ -f ${powershell} ]; then ${powershellInvocation}; else ${missingScriptFallback}; fi` + const windowsBranch = `if [ -f ${windowsScript} ]; then case "$HOME" in ${WINDOWS_GIT_BASH_RUNTIME_HOME_UNSAFE}) ${encodedWindowsBranch} ;; *) ${windowsScript} ;; esac; else ${missingScriptFallback}; fi` + const posixBranch = `if [ -f ${posixScript} ] && [ -r ${posixScript} ] && [ -x ${posixScript} ]; then /bin/sh ${posixScript}; else ${missingScriptFallback}; fi` // Why: OSTYPE is shell-owned, so platform selection adds no process to every hook invocation. - return `if [ -z "\${HOME-}" ]; then ${drain}; else case "\${OSTYPE-}" in msys*|cygwin*|win32*) ${windowsBranch} ;; *) ${posixBranch} ;; esac; fi` + return `if [ -z "\${HOME-}" ]; then ${missingScriptFallback}; else case "\${OSTYPE-}" in msys*|cygwin*|win32*) ${windowsBranch} ;; *) ${posixBranch} ;; esac; fi` } diff --git a/src/main/agent-hooks/windows-powershell-hook-launcher.ts b/src/main/agent-hooks/windows-powershell-hook-launcher.ts new file mode 100644 index 00000000000..e3974ef8ff1 --- /dev/null +++ b/src/main/agent-hooks/windows-powershell-hook-launcher.ts @@ -0,0 +1,27 @@ +// Why: centralizing the launcher keeps window suppression consistent across installers (#14815). + +// Why: an absolute forward-slash path avoids PATH hijacking and survives cmd.exe and Git Bash. +export function getWindowsSystem32Path(relativePath: string): string { + const systemRoot = process.env.SystemRoot || 'C:\\Windows' + return `${systemRoot.replaceAll('\\', '/')}/System32/${relativePath}` +} + +export function getWindowsPowerShellExecutablePath(): string { + return getWindowsSystem32Path('WindowsPowerShell/v1.0/powershell.exe') +} + +// Why: unlike conhost, hidden PowerShell relays hook output and exit status (#14818). +export const WINDOWS_POWERSHELL_HOOK_SWITCHES = + '-NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden' + +// Why: redirected PowerShell progress becomes CLIXML that can corrupt merged JSON output. +const HOOK_PROGRESS_SILENCER = "$ProgressPreference='SilentlyContinue'; " + +// Why: encoding shields paths and switches from cmd.exe and MSYS rewriting (#6078, #14815). +export function encodeWindowsPowerShellHookCommand(command: string): string { + return Buffer.from(`${HOOK_PROGRESS_SILENCER}${command}`, 'utf16le').toString('base64') +} + +export function wrapWindowsPowerShellEncodedCommand(command: string): string { + return `${getWindowsPowerShellExecutablePath()} ${WINDOWS_POWERSHELL_HOOK_SWITCHES} -EncodedCommand ${encodeWindowsPowerShellHookCommand(command)}` +} diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index b3fa02debc5..6f8eafb46e3 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -34,14 +34,28 @@ function hasManagedCommand(hook: TestHook, matcher: (command: string | undefined } describe('getWindowsManagedLifecycleHook', () => { - it('resolves the managed script from the runtime Windows profile', () => { + it('resolves the managed script from the runtime Windows profile, as a single command string', () => { const scriptPath = 'C:\\Users\\%name%\\a^b&c\\.orca\\agent-hooks\\claude-hook.cmd' const hook = getWindowsManagedLifecycleHook(scriptPath) - expect(hook.args?.[0]).toBe('--headless') - expect(hook.args?.[1]).toMatch(/\\System32\\cmd\.exe$/i) - expect(hook.args?.at(-1)).toBe('%USERPROFILE%\\.orca\\agent-hooks\\claude-hook.cmd') - expect(hook.args).not.toContain(scriptPath) + expect(hook.args).toBeUndefined() + expect(hook.command).toMatch( + /\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden / + ) + expect(hook.command).not.toContain(scriptPath) + // Why: Git Bash/MSYS mangles backslash paths and slash-prefixed switches. + expect(hook.command.replace(/-EncodedCommand \S+$/, '')).not.toMatch(/\\| \/[a-zA-Z]+( |$)/) + + const encoded = hook.command.match(/-EncodedCommand (\S+)$/)?.[1] + const decoded = Buffer.from(encoded ?? '', 'base64').toString('utf16le') + expect(decoded).toContain('$env:USERPROFILE') + expect(decoded).toContain('.orca\\agent-hooks\\claude-hook.cmd') + }) + + it('is still recognized as managed by createManagedCommandMatcher (#14825)', () => { + const scriptPath = 'C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd' + const hook = getWindowsManagedLifecycleHook(scriptPath) + expect(isClaudeManagedCommand(hook.command)).toBe(true) }) }) @@ -199,9 +213,17 @@ describe('ClaudeHookService.install', () => { expect(hasManagedCommand(legacy.hooks.StopFailure[0].hooks[0], isClaudeManagedCommand)).toBe( true ) - expect( - readFileSync(join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME), 'utf-8') - ).toContain('DEVIN_PROJECT_DIR') + const managedScript = readFileSync( + join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME), + 'utf-8' + ) + expect(managedScript).toContain('DEVIN_PROJECT_DIR') + // Why: guard and Devin-skip paths must still return neutral JSON (#14818). + expect(managedScript).toMatch( + process.platform === 'win32' + ? /^@echo off\r\nsetlocal\r\necho \{\}\r\n/ + : /^#!\/bin\/sh\nprintf "\{\}\\n"\n/ + ) } finally { vi.unstubAllEnvs() rmSync(tmpHome, { recursive: true, force: true }) @@ -255,7 +277,12 @@ describe('ClaudeHookService.install', () => { mkdirSync(join(tmpHome, '.claude'), { recursive: true }) writeFileSync( settingsPath, - JSON.stringify({ statusLine: { type: 'command', command: '/usr/local/bin/my-statusline' } }) + JSON.stringify({ + statusLine: { + type: 'command', + command: '/usr/local/bin/my-statusline' + } + }) ) expect(new ClaudeHookService().install().state).toBe('installed') @@ -339,7 +366,7 @@ describe('ClaudeHookService.install', () => { }) it.skipIf(process.platform !== 'win32')( - 'runs portable managed hooks through headless exec form', + 'runs portable managed hooks through a single headless command string', () => { const tmpHome = mkdtempSync(join(tmpdir(), 'orca claude home with spaces ')) vi.stubEnv('HOME', tmpHome) @@ -351,24 +378,20 @@ describe('ClaudeHookService.install', () => { readFileSync(join(tmpHome, '.claude', 'settings.json'), 'utf-8') ) as { hooks: Record } - const system32 = join(process.env.SystemRoot ?? 'C:\\Windows', 'System32') const scriptPath = join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME) - const runtimeScriptPath = join( - '%USERPROFILE%', - '.orca', - 'agent-hooks', - CLAUDE_SCRIPT_FILE_NAME - ) for (const eventName of ['UserPromptSubmit', 'Stop', 'StopFailure']) { const hook = settings.hooks[eventName]?.[0]?.hooks?.[0] - expect(hook).toEqual({ - type: 'command', - command: join(system32, 'conhost.exe'), - args: ['--headless', join(system32, 'cmd.exe'), '/d', '/c', runtimeScriptPath], - timeout: 10 - }) - expect(hook.args).not.toContain(scriptPath) + expect(hook?.args).toBeUndefined() + expect(hook?.command).toMatch( + /\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden / + ) + expect(hook?.command).not.toContain(scriptPath) + + const encoded = hook?.command.match(/-EncodedCommand (\S+)$/)?.[1] + const decoded = Buffer.from(encoded ?? '', 'base64').toString('utf16le') + expect(decoded).toContain('$env:USERPROFILE') + expect(decoded).toContain(`.orca\\agent-hooks\\${CLAUDE_SCRIPT_FILE_NAME}`) } } finally { vi.unstubAllEnvs() @@ -393,6 +416,8 @@ describe('ClaudeHookService.install', () => { expect(script).toContain('--data-urlencode "payload@-"') expect(script).toContain('/hook/claude') expect(script).not.toMatch(/Invoke-WebRequest/i) + // Why: guard and Devin-skip paths must still return neutral JSON (#14818). + expect(script.split('\r\n')[2]).toBe('echo {}') } finally { vi.unstubAllEnvs() rmSync(tmpHome, { recursive: true, force: true }) @@ -436,6 +461,10 @@ describe('ClaudeHookService.installRemote', () => { const script = fs.files.get('/home/dev/.orca/agent-hooks/claude-hook.sh') expect(script).toContain('#!/bin/sh') expect(script).toContain('DEVIN_PROJECT_DIR') + // Why: remote guard paths must still return neutral JSON (#14818). + expect(script!.indexOf('printf "{}\\n"')).toBe( + script!.indexOf('#!/bin/sh') + '#!/bin/sh\n'.length + ) // Why: payload is piped to curl via stdin (`payload@-`) so it never lands // on the curl command line (EDR oversized-command-line false positive), // matching the Windows curl.exe hook post. diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index 47269b1e17d..57270575c5c 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -65,6 +65,8 @@ function getManagedScript( return [ '@echo off', 'setlocal', + // Why: Claude-compatible permission hooks fail closed on empty stdout (#14818). + 'echo {}', // Why: refresh endpoint coordinates for PTYs surviving an Orca restart. 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', // Why (#11549): the env guards must outrank the Devin skip — the Devin skip parks in more.com, @@ -86,6 +88,8 @@ function getManagedScript( return [ '#!/bin/sh', + // Why: Claude-compatible permission hooks fail closed on empty stdout (#14818). + 'printf "{}\\n"', ...buildPosixHookPayloadCapture(), ...(options.skipWhenDevinImportsClaude ? [ diff --git a/src/main/claude/hook-settings.ts b/src/main/claude/hook-settings.ts index d39b8e27bcb..3447c62ffa5 100644 --- a/src/main/claude/hook-settings.ts +++ b/src/main/claude/hook-settings.ts @@ -6,7 +6,9 @@ import { getSharedManagedScriptPath, isPlainObject, MANAGED_HOOK_TIMEOUT_SECONDS, + quotePowerShellString, removeManagedCommands, + wrapWindowsPowerShellEncodedCommand, type HookCommandConfig, type HookDefinition, type HooksConfig @@ -16,38 +18,59 @@ import { wrapRuntimeHomeHookCommand } from '../agent-hooks/runtime-home-hook-com export type ClaudeCompatibleHookSettings = { configDirName: '.claude' | '.openclaude' scriptBaseName: 'claude-hook' | 'openclaude-hook' - supportsExecHookArgs: boolean + usesWindowsPowerShellLauncher: boolean } export const CLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.claude', scriptBaseName: 'claude-hook', - supportsExecHookArgs: true + usesWindowsPowerShellLauncher: true } export const OPENCLAUDE_HOOK_SETTINGS: ClaudeCompatibleHookSettings = { configDirName: '.openclaude', scriptBaseName: 'openclaude-hook', - supportsExecHookArgs: false + usesWindowsPowerShellLauncher: false } export const CLAUDE_EVENTS = [ // Why: SessionStart is the only event a resumed/idle session emits before the // first prompt; without it the sidebar row can't exist until the user types (STA-3386). - { eventName: 'SessionStart', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'UserPromptSubmit', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'Stop', definition: { hooks: [{ type: 'command', command: '' }] } }, + { + eventName: 'SessionStart', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'UserPromptSubmit', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'Stop', + definition: { hooks: [{ type: 'command', command: '' }] } + }, // Why: OpenClaude skips normal Stop hooks after API/model errors and emits // StopFailure instead; without this hook Orca leaves the turn spinning. - { eventName: 'StopFailure', definition: { hooks: [{ type: 'command', command: '' }] } }, + { + eventName: 'StopFailure', + definition: { hooks: [{ type: 'command', command: '' }] } + }, // Why: subagent/teammate lifecycle feeds the sidebar's child rows and keeps // a pane 'working' while background children outlive the lead's turn. // TeammateIdle parks turn-based teammates without trusting their permanently // "running" background_tasks entry to gate the pane. // Older Claude builds ignore unregistered event names (StopFailure precedent). - { eventName: 'SubagentStart', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'SubagentStop', definition: { hooks: [{ type: 'command', command: '' }] } }, - { eventName: 'TeammateIdle', definition: { hooks: [{ type: 'command', command: '' }] } }, + { + eventName: 'SubagentStart', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'SubagentStop', + definition: { hooks: [{ type: 'command', command: '' }] } + }, + { + eventName: 'TeammateIdle', + definition: { hooks: [{ type: 'command', command: '' }] } + }, // Why: PreToolUse gives the dashboard a live readout of the in-flight tool // (name + input preview) before it completes. { @@ -108,11 +131,15 @@ export function getRemoteConfigPath(remoteHome: string, settings = CLAUDE_HOOK_S return `${remoteHome.replace(/\/$/, '')}/${settings.configDirName}/settings.json` } -export function getManagedCommand(scriptPath: string): string { +export function getManagedCommand( + scriptPath: string, + options: { neutralJsonWhenMissing?: boolean } = {} +): string { const scriptFileName = basename(scriptPath) const extension = extname(scriptFileName) return wrapRuntimeHomeHookCommand( - extension ? scriptFileName.slice(0, -extension.length) : scriptFileName + extension ? scriptFileName.slice(0, -extension.length) : scriptFileName, + options ) } @@ -120,25 +147,25 @@ export function getManagedLifecycleHook( scriptPath: string, settings = CLAUDE_HOOK_SETTINGS ): HookCommandConfig { - if (process.platform !== 'win32' || !settings.supportsExecHookArgs) { - return buildManagedCommandHook(getManagedCommand(scriptPath)) + if (process.platform !== 'win32' || !settings.usesWindowsPowerShellLauncher) { + return buildManagedCommandHook(getManagedCommand(scriptPath, { neutralJsonWhenMissing: true })) } return getWindowsManagedLifecycleHook(scriptPath) } +// Why: some Claude-compatible consumers ignore `args`, so the invocation must be self-contained. export function getWindowsManagedLifecycleHook(scriptPath: string): HookCommandConfig { - const system32 = win32.join(process.env.SystemRoot ?? 'C:\\Windows', 'System32') - const runtimeScriptPath = win32.join( - '%USERPROFILE%', - '.orca', - 'agent-hooks', - win32.basename(scriptPath) - ) - // Why: Claude's Windows shell form opens Git Bash consoles; exec form hosts the client in a windowless console. + const scriptFileName = win32.basename(scriptPath) + // Why: runtime profile resolution keeps the managed entry portable across users (STA-3348). + const quotedRelativePath = quotePowerShellString(`.orca\\agent-hooks\\${scriptFileName}`) + // Why: compat consumers require neutral JSON even when the managed script is missing (#14818). + const innerCommand = + `$scriptPath = Join-Path $env:USERPROFILE ${quotedRelativePath}; ` + + 'if (Test-Path -LiteralPath $scriptPath -PathType Leaf) { & $scriptPath; exit $LASTEXITCODE }; ' + + "[Console]::In.ReadToEnd() | Out-Null; Write-Output '{}'; exit 0" return { type: 'command', - command: win32.join(system32, 'conhost.exe'), - args: ['--headless', win32.join(system32, 'cmd.exe'), '/d', '/c', runtimeScriptPath], + command: wrapWindowsPowerShellEncodedCommand(innerCommand), timeout: MANAGED_HOOK_TIMEOUT_SECONDS } } @@ -154,7 +181,7 @@ export function hasSameManagedHookInvocation( } export function getRemoteManagedCommand(scriptPath: string): string { - return getManagedCommand(scriptPath) + return getManagedCommand(scriptPath, { neutralJsonWhenMissing: true }) } export function applyManagedHooks( diff --git a/src/main/codex/hook-service-managed-install.test.ts b/src/main/codex/hook-service-managed-install.test.ts index 4f53c51513f..22f6bf698c5 100644 --- a/src/main/codex/hook-service-managed-install.test.ts +++ b/src/main/codex/hook-service-managed-install.test.ts @@ -30,7 +30,7 @@ vi.mock('os', async (importOriginal) => { import { CodexHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ const homes = setupCodexHookHomes(homedirMock, getPathMock) @@ -235,24 +235,31 @@ describe('CodexHookService', () => { // would block the event loop and starve this handler, so the child is // spawned asynchronously while the server drains the request concurrently. let resolveReceived: (value: { headers: Record; body: string }) => void - const receivedPromise = new Promise<{ headers: Record; body: string }>( - (resolve) => { - resolveReceived = resolve - } - ) + const receivedPromise = new Promise<{ + headers: Record + body: string + }>((resolve) => { + resolveReceived = resolve + }) const server = createServer((req, res) => { const chunks: Buffer[] = [] req.on('data', (c: Buffer) => chunks.push(c)) req.on('end', () => { res.end('ok') - resolveReceived({ headers: req.headers, body: Buffer.concat(chunks).toString('utf-8') }) + resolveReceived({ + headers: req.headers, + body: Buffer.concat(chunks).toString('utf-8') + }) }) }) await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) const port = (server.address() as AddressInfo).port try { - const payload = JSON.stringify({ prompt: '你好世界', hook_event_name: 'UserPromptSubmit' }) + const payload = JSON.stringify({ + prompt: '你好世界', + hook_event_name: 'UserPromptSubmit' + }) // Why: this suite may run inside an Orca-launched terminal whose env // already carries ORCA_AGENT_HOOK_ENDPOINT/PORT/TOKEN. The managed // script sources that endpoint file, so leave it out or the hook posts diff --git a/src/main/command-code/hook-service.test.ts b/src/main/command-code/hook-service.test.ts index 6e294516324..ce59e20b056 100644 --- a/src/main/command-code/hook-service.test.ts +++ b/src/main/command-code/hook-service.test.ts @@ -21,7 +21,7 @@ vi.mock('os', async () => { import { CommandCodeHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('CommandCodeHookService', () => { let homeDir: string @@ -136,7 +136,10 @@ describe('CommandCodeHookService', () => { body += chunk }) req.on('end', () => { - requests.push({ body, token: req.headers['x-orca-agent-hook-token'] }) + requests.push({ + body, + token: req.headers['x-orca-agent-hook-token'] + }) res.statusCode = 204 res.end() }) diff --git a/src/main/cursor/hook-service.test.ts b/src/main/cursor/hook-service.test.ts index f861d4f2a66..b723d947db3 100644 --- a/src/main/cursor/hook-service.test.ts +++ b/src/main/cursor/hook-service.test.ts @@ -31,7 +31,7 @@ const CURSOR_EVENTS = [ const CURSOR_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'cursor-hook.cmd' : 'cursor-hook.sh' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('CursorHookService', () => { let homeDir: string diff --git a/src/main/droid/hook-service.test.ts b/src/main/droid/hook-service.test.ts index ef31754a7e7..0a65facab21 100644 --- a/src/main/droid/hook-service.test.ts +++ b/src/main/droid/hook-service.test.ts @@ -25,7 +25,7 @@ vi.mock('os', async () => { import { DroidHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('DroidHookService', () => { let homeDir: string diff --git a/src/main/gemini/hook-service.test.ts b/src/main/gemini/hook-service.test.ts index 1a80c1568bf..fc5a302a7d1 100644 --- a/src/main/gemini/hook-service.test.ts +++ b/src/main/gemini/hook-service.test.ts @@ -26,7 +26,7 @@ vi.mock('os', async (importOriginal) => { import { GeminiHookService } from './hook-service' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ describe('GeminiHookService', () => { let homeDir: string diff --git a/src/main/grok/hook-service.test.ts b/src/main/grok/hook-service.test.ts index 9a58aabaf24..f08779aed02 100644 --- a/src/main/grok/hook-service.test.ts +++ b/src/main/grok/hook-service.test.ts @@ -23,7 +23,7 @@ import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract' const GROK_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'grok-hook.cmd' : 'grok-hook.sh' const WINDOWS_POWERSHELL_LAUNCHER = - /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -EncodedCommand \S+$/ + /^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -ExecutionPolicy Bypass -WindowStyle Hidden -EncodedCommand \S+$/ type WindowsGrokHookRun = { status: number | null @@ -343,7 +343,11 @@ describe('GrokHookService', () => { `${JSON.stringify( { hooks: { - Notification: [{ hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] }] + Notification: [ + { + hooks: [{ type: 'command', command: '/usr/local/bin/user-hook' }] + } + ] } }, null, From 226cf88ba6c9d85f332b0b23a50b1e64c830ee69 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:49:45 -0700 Subject: [PATCH 5/8] fix(terminal): inset the grid inside the xterm surface (#14583) * fix(terminal): inset the grid inside the xterm surface (#13252) Padding X/Y was applied as start-edge container margin, so the cell grid stayed flush on the trailing edges and a fractional background opacity stacked a darker gutter around the viewport. Put the setting on .xterm so FitAddon insets both axes and the themed background fills the pad. * fix(terminal): normalize padding before fit * fix(terminal): align stored and fitted padding * test(terminal): lock padding before fit * fix terminal padding opacity compositing * fix live terminal padding backgrounds * fix(terminal): preserve source-over alpha blending * fix(terminal): restore WebGL alpha blending * test(terminal): complete hidden retention pane fixture --- src/main/ghostty/mapper-extended.test.ts | 4 +- src/main/ghostty/numeric-config-values.ts | 7 +- .../terminal-container-geometry.test.ts | 30 ++++++- src/renderer/src/assets/terminal.css | 50 ++++++++++-- .../TerminalWindowSection.padding.test.tsx | 81 +++++++++++++++++++ .../settings/TerminalWindowSection.tsx | 13 ++- .../terminal-pane/terminal-appearance.test.ts | 69 ++++++++++++++++ .../terminal-pane/terminal-appearance.ts | 30 ++++--- .../pane-background-compositing.test.ts | 57 +++++++++++++ .../pane-background-compositing.ts | 30 +++++++ .../lib/pane-manager/pane-lifecycle.test.ts | 1 + .../src/lib/pane-manager/pane-lifecycle.ts | 6 ++ .../pane-manager/pane-webgl-renderer.test.ts | 68 +++++++++++++++- .../lib/pane-manager/pane-webgl-renderer.ts | 48 +++++++++++ .../terminal-webgl-hidden-retention.test.ts | 1 + src/shared/terminal-padding-settings.test.ts | 14 ++++ src/shared/terminal-padding-settings.ts | 6 ++ 17 files changed, 481 insertions(+), 34 deletions(-) create mode 100644 src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx create mode 100644 src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts create mode 100644 src/renderer/src/lib/pane-manager/pane-background-compositing.ts create mode 100644 src/shared/terminal-padding-settings.test.ts create mode 100644 src/shared/terminal-padding-settings.ts diff --git a/src/main/ghostty/mapper-extended.test.ts b/src/main/ghostty/mapper-extended.test.ts index 402e9253f21..189e5bf4217 100644 --- a/src/main/ghostty/mapper-extended.test.ts +++ b/src/main/ghostty/mapper-extended.test.ts @@ -90,9 +90,9 @@ describe('mapGhosttyToOrca — window-padding', () => { expect(result.unsupportedKeys).toEqual([]) }) - it('preserves odd-sum dual-value padding as a fractional average', () => { + it('rounds odd-sum dual-value padding to the nearest fitted pixel', () => { const result = mapGhosttyToOrca({ 'window-padding-x': '1,2' }) - expect(result.diff).toEqual({ terminalPaddingX: 1.5 }) + expect(result.diff).toEqual({ terminalPaddingX: 2 }) expect(result.unsupportedKeys).toEqual([]) }) diff --git a/src/main/ghostty/numeric-config-values.ts b/src/main/ghostty/numeric-config-values.ts index f8a7ce7b468..5ced69afa48 100644 --- a/src/main/ghostty/numeric-config-values.ts +++ b/src/main/ghostty/numeric-config-values.ts @@ -1,3 +1,5 @@ +import { normalizeTerminalPadding } from '../../shared/terminal-padding-settings' + // Why: `Number("1e10")` succeeds and passes `Number.isInteger`, so a Ghostty // config with `window-padding-x = 1e9` would sail through the mapper and land // an absurd value in the store. Restrict to plain decimal integers. @@ -11,8 +13,7 @@ export const parseStrictInt = (v: string): number | null => { } // Why: Ghostty accepts "top,bottom" / "left,right" pairs for window paddings, -// but Orca stores a single value per axis — average the pair so the total -// padding along the axis stays the same. +// but Orca stores an integer per axis — preserve the nearest total inset. export const parsePaddingValue = (v: string): number | null => { const parts = v.split(',') if (parts.length > 2) { @@ -26,5 +27,5 @@ export const parsePaddingValue = (v: string): number | null => { } nums.push(num) } - return nums.reduce((sum, num) => sum + num, 0) / nums.length + return normalizeTerminalPadding(nums.reduce((sum, num) => sum + num, 0) / nums.length) } diff --git a/src/renderer/src/assets/terminal-container-geometry.test.ts b/src/renderer/src/assets/terminal-container-geometry.test.ts index db079c9095f..5e2bfcc0258 100644 --- a/src/renderer/src/assets/terminal-container-geometry.test.ts +++ b/src/renderer/src/assets/terminal-container-geometry.test.ts @@ -5,9 +5,7 @@ const terminalCss = fs.readFileSync(new URL('./terminal.css', import.meta.url), describe('terminal container geometry', () => { it('keeps the hidden link tooltip out of the fitted terminal height', () => { - expect(terminalCss).toMatch( - /\.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--pane-padding-y, 4px\)\);/s - ) + expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*height:\s*100%;/s) expect(terminalCss).toMatch( /\.pane\[data-has-title\] \.xterm-container\s*{[^}]*height:\s*calc\(100% - var\(--orca-pane-title-height\)\);/s ) @@ -15,4 +13,30 @@ describe('terminal container geometry', () => { /\.pane-link-tooltip\s*{[^}]*height:\s*var\(--orca-terminal-link-tooltip-height\);/s ) }) + + it('insets the xterm grid on both axes from pane padding', () => { + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-top:\s*var\(--pane-padding-y, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-right:\s*var\(--pane-padding-x, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-bottom:\s*var\(--pane-padding-y, 4px\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container \.xterm\s*{[^}]*padding-left:\s*var\(--pane-padding-x, 4px\);/s + ) + expect(terminalCss).toMatch(/\.xterm-container\s*{[^}]*width:\s*100%;/s) + expect(terminalCss).not.toMatch(/\.xterm-container\s*{[^}]*margin-left:/s) + }) + + it('matches live DOM and WebGL background layers across translucent padding bands', () => { + expect(terminalCss).toMatch( + /\.xterm-container \.xterm::before\s*{[^}]*border-color:\s*var\(--orca-terminal-live-background, transparent\);/s + ) + expect(terminalCss).toMatch( + /\.xterm-container\[data-terminal-renderer='webgl'\] \.xterm::after\s*{[^}]*border-color:\s*var\(--orca-terminal-live-background, transparent\);/s + ) + }) }) diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index ee5e0af01f5..66a8a18828a 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -30,6 +30,42 @@ width: 100%; } +.xterm-container .xterm { + box-sizing: border-box; + height: 100%; + width: 100%; + /* Why on .xterm, not the container: FitAddon subtracts this element's + padding when proposing cols/rows, and the themed background (including + opacity) fills the inset. Container margins only framed the start edges + and stacked a darker gutter at fractional opacity. */ + padding-top: var(--pane-padding-y, 4px); + padding-right: var(--pane-padding-x, 4px); + padding-bottom: var(--pane-padding-y, 4px); + padding-left: var(--pane-padding-x, 4px); +} + +/* xterm paints the live viewport background again under the grid. Mirror that + layer in the padding bands, including runtime OSC 11 color changes. */ +.xterm-container .xterm::before, +.xterm-container[data-terminal-renderer='webgl'] .xterm::after { + box-sizing: border-box; + position: absolute; + inset: 0; + content: ''; + pointer-events: none; + border-style: solid; + border-width: var(--pane-padding-y, 4px) var(--pane-padding-x, 4px); +} + +.xterm-container .xterm::before { + border-color: var(--orca-terminal-live-background, transparent); +} + +/* WebGL adds one source-over background layer beyond the DOM renderer. */ +.xterm-container[data-terminal-renderer='webgl'] .xterm::after { + border-color: var(--orca-terminal-live-background, transparent); +} + .pane-manager-root .xterm-viewport { /* Why: xterm/VS Code keep the scrollbar gutter stable. `auto` lets Linux DOM scrollbars appear/disappear, which can feed resize/reflow jitter. */ @@ -177,7 +213,7 @@ .pane[data-terminal-attention] .xterm-container::after { content: ''; position: absolute; - inset: 0 4px 4px 0; + inset: 0; z-index: 5; pointer-events: none; border-radius: 2px; @@ -498,16 +534,14 @@ border-color: var(--ring); } -/* Baseline xterm-container layout — padding is driven by CSS custom properties - so Ghostty-imported window-padding-x/y can be applied without inline styles. - The fallback 4px preserves the legacy default. */ +/* Container fills the pane so the .xterm background (and its padding) is the + terminal surface. Title offset stays here because the title overlay is a + sibling of the container, not of .xterm. */ .xterm-container { box-sizing: border-box; position: relative; - width: calc(100% - var(--pane-padding-x, 4px)); - height: calc(100% - var(--pane-padding-y, 4px)); - margin-top: var(--pane-padding-y, 4px); - margin-left: var(--pane-padding-x, 4px); + width: 100%; + height: 100%; } /* When a pane has a title, shift the terminal content down to make room. diff --git a/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx b/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx new file mode 100644 index 00000000000..5f54fb38a9e --- /dev/null +++ b/src/renderer/src/components/settings/TerminalWindowSection.padding.test.tsx @@ -0,0 +1,81 @@ +// @vitest-environment happy-dom + +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { getDefaultSettings } from '../../../../shared/constants' +import { TerminalWindowSection } from './TerminalWindowSection' + +type NumberFieldProps = { + label: string + value: number + onChange: (value: number) => void +} + +const numberFields = vi.hoisted(() => new Map()) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, defaultValue: string) => defaultValue +})) + +vi.mock('./SettingsFormControls', () => ({ + ColorField: function ColorField() { + return null + }, + NumberField: function NumberField(props: NumberFieldProps) { + numberFields.set(props.label, props) + return null + } +})) + +vi.mock('./SearchableSetting', () => ({ + SearchableSetting: function SearchableSetting({ children }: { children?: ReactNode }) { + return children ?? null + } +})) + +vi.mock('../ui/button', () => ({ Button: () => null })) +vi.mock('../ui/label', () => ({ Label: () => null })) +vi.mock('../ui/switch', () => ({ Switch: () => null })) + +describe('TerminalWindowSection padding', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + numberFields.clear() + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + document.body.replaceChildren() + }) + + it('displays and persists the same integer padding used by terminal fitting', () => { + const updateSettings = vi.fn() + const settings = { + ...getDefaultSettings('/tmp'), + terminalPaddingX: 1.5, + terminalPaddingY: 2.5 + } as GlobalSettings + + act(() => + root.render() + ) + + const horizontal = numberFields.get('Horizontal Padding') + const vertical = numberFields.get('Vertical Padding') + expect(horizontal?.value).toBe(2) + expect(vertical?.value).toBe(3) + + act(() => horizontal?.onChange(3.5)) + act(() => vertical?.onChange(4.5)) + expect(updateSettings).toHaveBeenNthCalledWith(1, { terminalPaddingX: 4 }) + expect(updateSettings).toHaveBeenNthCalledWith(2, { terminalPaddingY: 5 }) + }) +}) diff --git a/src/renderer/src/components/settings/TerminalWindowSection.tsx b/src/renderer/src/components/settings/TerminalWindowSection.tsx index 863d7fa11b0..5a09073cc27 100644 --- a/src/renderer/src/components/settings/TerminalWindowSection.tsx +++ b/src/renderer/src/components/settings/TerminalWindowSection.tsx @@ -1,6 +1,7 @@ import { useRef, useState } from 'react' import { RotateCw } from 'lucide-react' import type { GlobalSettings } from '../../../../shared/global-settings-types' +import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings' import { Button } from '../ui/button' import { Label } from '../ui/label' import { Switch } from '../ui/switch' @@ -185,13 +186,15 @@ export function TerminalWindowSection({ 'Horizontal Padding' )} description="" - value={settings.terminalPaddingX ?? 4} + value={normalizeTerminalPadding(settings.terminalPaddingX ?? 4)} defaultValue={4} min={0} max={512} step={1} suffix="px" - onChange={(value) => updateSettings({ terminalPaddingX: Math.max(0, value) })} + onChange={(value) => + updateSettings({ terminalPaddingX: normalizeTerminalPadding(value) }) + } /> @@ -212,13 +215,15 @@ export function TerminalWindowSection({ 'Vertical Padding' )} description="" - value={settings.terminalPaddingY ?? 4} + value={normalizeTerminalPadding(settings.terminalPaddingY ?? 4)} defaultValue={4} min={0} max={512} step={1} suffix="px" - onChange={(value) => updateSettings({ terminalPaddingY: Math.max(0, value) })} + onChange={(value) => + updateSettings({ terminalPaddingY: normalizeTerminalPadding(value) }) + } /> diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts index 6f7bec8592b..8998b6a43af 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.test.ts @@ -150,6 +150,7 @@ describe('applyTerminalAppearance theme assignment', () => { getBoundingClientRect: () => ({ width: measurable ? 800 : 0, height: measurable ? 600 : 0 }) }, fitAddon: { + fit: vi.fn(), proposeDimensions: () => (measurable ? { cols: 80, rows: 24 } : undefined) } } as unknown as ManagedPane @@ -360,6 +361,74 @@ describe('applyTerminalAppearance theme assignment', () => { // Latest wins, exactly one write: intermediate hidden values never touch xterm. expect(writes).toEqual([21]) }) + + it('stamps settings-UI padding defaults when padding is unset', () => { + const pane = makePane(1) + const manager = makeManager([pane]) + applyTerminalAppearance( + manager, + getDefaultSettings('/tmp'), + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + expect(manager.setPaneStyleOptions).toHaveBeenCalledWith( + expect.objectContaining({ paddingX: 4, paddingY: 4 }) + ) + }) + + it('rounds imported half-pixel padding before fitting', () => { + const pane = makePane(1) + const manager = makeManager([pane]) + const settings = { + ...getDefaultSettings('/tmp'), + terminalPaddingX: 1.5, + terminalPaddingY: 2.5 + } + + applyTerminalAppearance( + manager, + settings, + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + + expect(manager.setPaneStyleOptions).toHaveBeenCalledWith( + expect.objectContaining({ paddingX: 2, paddingY: 3 }) + ) + }) + + it('stamps padding before fitting the pane', () => { + const pane = makePane(1) + pane.fitAddon.proposeDimensions = () => ({ cols: 79, rows: 23 }) + const manager = makeManager([pane]) + + applyTerminalAppearance( + manager, + getDefaultSettings('/tmp'), + true, + new Map(), + new Map(), + 'false', + new Map(), + new Map() + ) + + const setPaneStyleOptions = vi.mocked(manager.setPaneStyleOptions) + const fit = vi.mocked(pane.fitAddon.fit) + expect(setPaneStyleOptions).toHaveBeenCalledOnce() + expect(fit).toHaveBeenCalledOnce() + expect(setPaneStyleOptions.mock.invocationCallOrder[0]!).toBeLessThan( + fit.mock.invocationCallOrder[0]! + ) + }) }) describe('publishTerminalViewAttributesAtAppStart', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-appearance.ts b/src/renderer/src/components/terminal-pane/terminal-appearance.ts index bb61a50fbbc..f520d98baba 100644 --- a/src/renderer/src/components/terminal-pane/terminal-appearance.ts +++ b/src/renderer/src/components/terminal-pane/terminal-appearance.ts @@ -3,6 +3,7 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' import type { GlobalSettings } from '../../../../shared/global-settings-types' import { resolveTerminalFontWeights } from '../../../../shared/terminal-fonts' import { resolveTerminalLigaturesEnabled } from '../../../../shared/terminal-ligatures' +import { normalizeTerminalPadding } from '../../../../shared/terminal-padding-settings' import { getBuiltinTheme, resolvePaneStyleOptions, @@ -158,6 +159,23 @@ export function applyTerminalAppearance( settings.terminalLigatures, settings.terminalFontFamily ) + // FitAddon parses each CSS padding as an integer, so normalize imported half-pixels before styling and fitting. + const paddingX = normalizeTerminalPadding(settings.terminalPaddingX ?? 4) + const paddingY = normalizeTerminalPadding(settings.terminalPaddingY ?? 4) + + // Why before the pane loop: FitAddon subtracts live .xterm padding, so the + // CSS vars must be stamped before safeFit or a padding shrink leaves a stale grid. + manager.setPaneStyleOptions({ + splitBackground: paneBackground, + paneBackground, + inactivePaneOpacity: paneStyles.inactivePaneOpacity, + activePaneOpacity: paneStyles.activePaneOpacity, + opacityTransitionMs: paneStyles.opacityTransitionMs, + dividerThicknessPx: paneStyles.dividerThicknessPx, + focusFollowsMouse: paneStyles.focusFollowsMouse, + paddingX, + paddingY + }) for (const pane of manager.getPanes()) { // Why value-gated: writing options.theme rebuilds the palette, discarding TUI OSC 4/10/11/12 mutations; skip on no-op change. @@ -227,16 +245,4 @@ export function applyTerminalAppearance( safeFit(pane) } } - - manager.setPaneStyleOptions({ - splitBackground: paneBackground, - paneBackground, - inactivePaneOpacity: paneStyles.inactivePaneOpacity, - activePaneOpacity: paneStyles.activePaneOpacity, - opacityTransitionMs: paneStyles.opacityTransitionMs, - dividerThicknessPx: paneStyles.dividerThicknessPx, - focusFollowsMouse: paneStyles.focusFollowsMouse, - paddingX: settings.terminalPaddingX, - paddingY: settings.terminalPaddingY - }) } diff --git a/src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts b/src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts new file mode 100644 index 00000000000..c7540a68c1d --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-background-compositing.test.ts @@ -0,0 +1,57 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ManagedPaneInternal } from './pane-manager-types' +import { + disposePaneTerminalBackgroundObserver, + observePaneTerminalBackground +} from './pane-background-compositing' + +const observers: { callback: MutationCallback; disconnect: ReturnType }[] = [] + +beforeEach(() => { + observers.length = 0 + vi.stubGlobal( + 'MutationObserver', + class { + readonly disconnect = vi.fn() + + constructor(readonly callback: MutationCallback) { + observers.push(this) + } + + observe(): void {} + } + ) +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('terminal padding background compositing', () => { + it('tracks live xterm background changes and releases the observer', () => { + const terminalElement = { style: { backgroundColor: 'rgba(0, 0, 0, 0.5)' } } + const setProperty = vi.fn() + const pane = { + terminal: { element: terminalElement }, + xtermContainer: { + style: { setProperty } + } + } as unknown as ManagedPaneInternal + + observePaneTerminalBackground(pane) + expect(setProperty).toHaveBeenCalledWith( + '--orca-terminal-live-background', + 'rgba(0, 0, 0, 0.5)' + ) + + terminalElement.style.backgroundColor = 'rgb(255, 0, 0)' + observers[0]?.callback([], observers[0] as unknown as MutationObserver) + expect(setProperty).toHaveBeenLastCalledWith( + '--orca-terminal-live-background', + 'rgb(255, 0, 0)' + ) + + disposePaneTerminalBackgroundObserver(pane) + expect(observers[0]?.disconnect).toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-background-compositing.ts b/src/renderer/src/lib/pane-manager/pane-background-compositing.ts new file mode 100644 index 00000000000..754315ae751 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-background-compositing.ts @@ -0,0 +1,30 @@ +import type { ManagedPaneInternal } from './pane-manager-types' + +const observers = new WeakMap() + +export function observePaneTerminalBackground(pane: ManagedPaneInternal): void { + disposePaneTerminalBackgroundObserver(pane) + const terminalElement = pane.terminal.element + if (!terminalElement) { + return + } + + let previous = '' + const sync = () => { + const background = terminalElement.style.backgroundColor + if (!background || background === previous) { + return + } + previous = background + pane.xtermContainer.style.setProperty('--orca-terminal-live-background', background) + } + const observer = new MutationObserver(sync) + observer.observe(terminalElement, { attributes: true, attributeFilter: ['style'] }) + observers.set(pane, observer) + sync() +} + +export function disposePaneTerminalBackgroundObserver(pane: ManagedPaneInternal): void { + observers.get(pane)?.disconnect() + observers.delete(pane) +} diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index 9cb442a2073..89544daafca 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -497,6 +497,7 @@ describe('openTerminal — addon and provider wiring', () => { addEventListener: vi.fn(), removeEventListener: vi.fn(), querySelector: vi.fn(() => null), + style: { backgroundColor: '' }, classList: { contains: vi.fn(() => false) } } as unknown as HTMLElement vi.stubGlobal( diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index 3c89a6ec723..079925d1826 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -20,6 +20,10 @@ import { rebuildAttachedWebgl } from './pane-webgl-reattach' import { configureLazyArabicShapingJoiner } from './terminal-arabic-shaping-joiner' import { TerminalLigaturesAddon } from './terminal-ligatures-addon' import { installTerminalImeCandidateAnchor } from './terminal-ime-candidate-anchor' +import { + disposePaneTerminalBackgroundObserver, + observePaneTerminalBackground +} from './pane-background-compositing' // --------------------------------------------------------------------------- // Pane creation, terminal open/close, addon management @@ -44,6 +48,7 @@ export function openTerminal(pane: ManagedPaneInternal): void { // Open terminal into DOM terminal.open(xtermContainer) + observePaneTerminalBackground(pane) // Why: terminal.element sits under the padded xterm container. Pane-level // placement keeps the hover URL on the true bottom-left window corner. container.appendChild(linkTooltip) @@ -175,6 +180,7 @@ export function disposePane( pane.pendingInitialFitRafId = null } cancelPendingWebglRefresh(pane) + disposePaneTerminalBackgroundObserver(pane) detachPaneFitResizeObserver(pane) if (pane.panePointerDownHandler) { pane.container.removeEventListener('pointerdown', pane.panePointerDownHandler) diff --git a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts index b3990f20400..ddaf330a48d 100644 --- a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.test.ts @@ -4,6 +4,7 @@ import type { ManagedPaneInternal } from './pane-manager-types' import { attachWebgl, clearTerminalWebglAttachBackoff, + disposeWebgl, resetTerminalWebglSuggestion, resetWebglTextureAtlas } from './pane-webgl-renderer' @@ -11,7 +12,7 @@ import { notifyPaneFitSucceeded } from './pane-fit-webgl-attach-signal' import { safeFit } from './pane-fit' import { disposePane } from './pane-lifecycle' -function createPane(options: { loadAddon?: () => void } = {}): ManagedPaneInternal { +function createPane(options: { loadAddon?: (addon: unknown) => void } = {}): ManagedPaneInternal { const leafId = '22222222-2222-4222-8222-222222222222' as never return { id: 1, @@ -24,7 +25,7 @@ function createPane(options: { loadAddon?: () => void } = {}): ManagedPaneIntern loadAddon: vi.fn(options.loadAddon) } as never, container: {} as never, - xtermContainer: {} as never, + xtermContainer: { dataset: {} } as never, linkTooltip: {} as never, terminalGpuAcceleration: 'on', gpuRenderingEnabled: true, @@ -95,6 +96,69 @@ describe('terminal WebGL addon lifecycle', () => { expect(pane.webglAddon).toBeNull() }) + it('tracks the active renderer for WebGL padding compositing', () => { + const pane = createPane() + + attachWebgl(pane) + expect(pane.xtermContainer.dataset.terminalRenderer).toBe('webgl') + + disposeWebgl(pane) + expect(pane.xtermContainer.dataset.terminalRenderer).toBe('dom') + }) + + it('uses source-over alpha blending for colored translucent backgrounds', () => { + const blendFuncSeparate = vi.fn() + const gl = { + SRC_ALPHA: 1, + ONE_MINUS_SRC_ALPHA: 2, + ONE: 3, + blendFuncSeparate + } + const pane = createPane({ + loadAddon: (addon) => { + ;(addon as { _renderer: { _gl: typeof gl } })._renderer = { _gl: gl } + } + }) + + attachWebgl(pane) + + expect(blendFuncSeparate).toHaveBeenCalledWith(1, 2, 3, 2) + }) + + it('restores source-over alpha blending after WebGL context restoration', () => { + const blendFuncSeparate = vi.fn() + const canvas = new EventTarget() + const gl = { + SRC_ALPHA: 1, + ONE_MINUS_SRC_ALPHA: 2, + ONE: 3, + blendFuncSeparate + } + const pane = createPane({ + loadAddon: (addon) => { + canvas.addEventListener('webglcontextrestored', () => blendFuncSeparate(4, 5, 6, 7)) + ;(addon as { _renderer: { _canvas: EventTarget; _gl: typeof gl } })._renderer = { + _canvas: canvas, + _gl: gl + } + } + }) + + attachWebgl(pane) + blendFuncSeparate.mockClear() + canvas.dispatchEvent(new Event('webglcontextrestored')) + + expect(blendFuncSeparate).toHaveBeenCalledTimes(2) + expect(blendFuncSeparate).toHaveBeenNthCalledWith(1, 4, 5, 6, 7) + expect(blendFuncSeparate).toHaveBeenLastCalledWith(1, 2, 3, 2) + + disposeWebgl(pane) + blendFuncSeparate.mockClear() + canvas.dispatchEvent(new Event('webglcontextrestored')) + expect(blendFuncSeparate).toHaveBeenCalledOnce() + expect(blendFuncSeparate).toHaveBeenCalledWith(4, 5, 6, 7) + }) + it('disposes the previous addon before attaching a replacement', () => { const pane = createPane() attachWebgl(pane) diff --git a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts index f162052cebe..488bf32c94c 100644 --- a/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts +++ b/src/renderer/src/lib/pane-manager/pane-webgl-renderer.ts @@ -24,6 +24,10 @@ let suggestedRendererType: 'dom' | undefined type ReleasableWebglContext = { getExtension(name: 'WEBGL_lose_context'): WEBGL_lose_context | null isContextLost?: () => boolean + blendFuncSeparate?: (srcRgb: number, dstRgb: number, srcAlpha: number, dstAlpha: number) => void + readonly SRC_ALPHA: number + readonly ONE_MINUS_SRC_ALPHA: number + readonly ONE: number } type XtermWebglAddonInternals = { @@ -33,6 +37,11 @@ type XtermWebglAddonInternals = { } } +const webglContextRestoreHandlers = new WeakMap< + WebglAddon, + { canvas: HTMLCanvasElement; handler: EventListener } +>() + export function resetTerminalWebglSuggestion(): void { // Why: toggling GPU settings should let "auto" retry WebGL after an earlier // attach failure suggested DOM rendering for this app session. Per-pane @@ -89,6 +98,7 @@ export function disposeWebgl( options?: { refreshDimensions?: boolean } ): void { cancelPendingWebglRefresh(pane) + markPaneTerminalRenderer(pane, 'dom') if (!pane.webglAddon) { return } @@ -118,6 +128,7 @@ export function disposeWebgl( } function releaseXtermWebglContext(webglAddon: ManagedPaneInternal['webglAddon']): void { + removeWebglContextRestoreHandler(webglAddon) try { // Why: xterm removes the canvas on dispose, but Windows/ANGLE can keep the // driver context alive long enough for rapid terminal activation to hit @@ -259,7 +270,10 @@ export function attachWebgl(pane: ManagedPaneInternal): void { disposeWebgl(pane, { refreshDimensions: true }) }) pane.terminal.loadAddon(addon) + normalizeWebglAlphaBlending(addon) + installWebglContextRestoreHandler(addon) pane.webglAddon = addon + markPaneTerminalRenderer(pane, 'webgl') refreshTerminalAfterWebglAttach(pane) } catch (err) { if (pane.terminalGpuAcceleration === 'auto') { @@ -278,3 +292,37 @@ export function attachWebgl(pane: ManagedPaneInternal): void { pane.webglAddon = null } } + +function normalizeWebglAlphaBlending(webglAddon: WebglAddon): void { + const gl = (webglAddon as unknown as XtermWebglAddonInternals)._renderer?._gl + // xterm's shared blendFunc squares destination alpha. Standard source-over keeps colored translucent backgrounds pixel-identical to CSS. + gl?.blendFuncSeparate?.(gl.SRC_ALPHA, gl.ONE_MINUS_SRC_ALPHA, gl.ONE, gl.ONE_MINUS_SRC_ALPHA) +} + +function installWebglContextRestoreHandler(webglAddon: WebglAddon): void { + const canvas = (webglAddon as unknown as XtermWebglAddonInternals)._renderer?._canvas + if (!canvas) { + return + } + const handler = () => normalizeWebglAlphaBlending(webglAddon) + canvas.addEventListener('webglcontextrestored', handler) + webglContextRestoreHandlers.set(webglAddon, { canvas, handler }) +} + +function removeWebglContextRestoreHandler(webglAddon: ManagedPaneInternal['webglAddon']): void { + if (!webglAddon) { + return + } + const registered = webglContextRestoreHandlers.get(webglAddon) + if (!registered) { + return + } + registered.canvas.removeEventListener('webglcontextrestored', registered.handler) + webglContextRestoreHandlers.delete(webglAddon) +} + +function markPaneTerminalRenderer(pane: ManagedPaneInternal, renderer: 'dom' | 'webgl'): void { + if (pane.xtermContainer.dataset) { + pane.xtermContainer.dataset.terminalRenderer = renderer + } +} diff --git a/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts b/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts index cfe3fba4e37..4b749c50e75 100644 --- a/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts +++ b/src/renderer/src/lib/pane-manager/terminal-webgl-hidden-retention.test.ts @@ -10,6 +10,7 @@ import { function createPane(withAddon = true): ManagedPaneInternal { return { + xtermContainer: { dataset: {} }, terminal: { blur: vi.fn() }, webglAddon: withAddon ? ({ dispose: vi.fn() } as unknown as ManagedPaneInternal['webglAddon']) diff --git a/src/shared/terminal-padding-settings.test.ts b/src/shared/terminal-padding-settings.test.ts new file mode 100644 index 00000000000..ee7ac432f46 --- /dev/null +++ b/src/shared/terminal-padding-settings.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from 'vitest' +import { normalizeTerminalPadding } from './terminal-padding-settings' + +describe('normalizeTerminalPadding', () => { + it('rounds fractional padding to the nearest fitted pixel', () => { + expect(normalizeTerminalPadding(1.5)).toBe(2) + expect(normalizeTerminalPadding(2.49)).toBe(2) + }) + + it('keeps padding inside the settings range', () => { + expect(normalizeTerminalPadding(-1)).toBe(0) + expect(normalizeTerminalPadding(513)).toBe(512) + }) +}) diff --git a/src/shared/terminal-padding-settings.ts b/src/shared/terminal-padding-settings.ts new file mode 100644 index 00000000000..1f18a46973f --- /dev/null +++ b/src/shared/terminal-padding-settings.ts @@ -0,0 +1,6 @@ +const MIN_TERMINAL_PADDING = 0 +const MAX_TERMINAL_PADDING = 512 + +export function normalizeTerminalPadding(value: number): number { + return Math.min(MAX_TERMINAL_PADDING, Math.max(MIN_TERMINAL_PADDING, Math.round(value))) +} From 8b6d0231b2b98317a95bb64c2951811228e1e6ff Mon Sep 17 00:00:00 2001 From: hwantage <82494320+hwantage@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:49:59 +0900 Subject: [PATCH 6/8] feat(i18n): localize Automations settings and navigation to Korean (#15004) --- src/renderer/src/i18n/locales/ko.json | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 3060a5b34c0..adc869d8eb4 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -825,7 +825,9 @@ "linearDescription": "Orca에서 Linear가 작동하는 방식, 설정 체크리스트, 에이전트 스킬, 프롬프트 예시입니다.", "pluginsTitle": "플러그인", "pluginsDescription": "실험적인 Orca 플러그인을 설치하고 관리합니다.", - "tasksDescription": "제공업체를 연결하고 Linear 스킬을 설치한 다음 작업에 표시할 항목을 선택하세요." + "tasksDescription": "제공업체를 연결하고 Linear 스킬을 설치한 다음 작업에 표시할 항목을 선택하세요.", + "automationsTitle": "자동화", + "automationsDescription": "에이전트 작업을 예약하고 사이드바에 자동화 표시 여부를 선택합니다." }, "useAppMenuPaste": { "pasteTooLarge": "붙여넣기 내용이 너무 큽니다." @@ -10144,6 +10146,22 @@ "saveFailed": "가시성 기본값을 저장할 수 없습니다.", "updateServer": "소스 기본값을 구성하려면 이 서버를 업데이트하세요.", "updateServerDefaults": "표시 기본값을 구성하려면 이 서버를 업데이트하세요." + }, + "automations": { + "title": "자동화", + "description": "에이전트 작업을 예약하고 사이드바에 자동화 표시 여부를 선택합니다.", + "showButton": "자동화 버튼 표시", + "showButtonDescription": "사이드바에 자동화 바로 가기를 표시합니다.", + "howItWorksTitle": "자동화 작동 방식", + "howItWorksDescription": "에이전트 작업을 한 번만 예약하면 Orca가 각 실행을 생성하고 결과를 한곳에 모아 관리합니다.", + "defineStepTitle": "작업 설명", + "defineStepDescription": "프로젝트, 에이전트, 프롬프트 및 일정을 선택합니다.", + "runStepTitle": "Orca가 각 실행 시작", + "runStepDescription": "예약 시간이 되면 선택된 에이전트에게 새로운 작업 공간이 할당됩니다.", + "reviewStepTitle": "결과 검토", + "reviewStepDescription": "최근 실행을 검토하고 필요할 때 언제든지 작업을 계속할 수 있습니다.", + "openAutomations": "자동화 열기", + "openAutomationsDescription": "일정을 생성하고 최근 실행을 확인합니다." } }, "right": { From 88b1a698240c619a792c27b541f2ae2b5f925922 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:57:11 -0700 Subject: [PATCH 7/8] Fix Windows horizontal computer-use scroll (#14727) --- ...ter-use-windows-horizontal-scroll.test.mjs | 48 +++++++++++++++++++ native/computer-use-windows/runtime.ps1 | 13 +++-- 2 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 config/scripts/computer-use-windows-horizontal-scroll.test.mjs diff --git a/config/scripts/computer-use-windows-horizontal-scroll.test.mjs b/config/scripts/computer-use-windows-horizontal-scroll.test.mjs new file mode 100644 index 00000000000..3d76908daac --- /dev/null +++ b/config/scripts/computer-use-windows-horizontal-scroll.test.mjs @@ -0,0 +1,48 @@ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' + +const projectDir = resolve(import.meta.dirname, '../..') + +function source(path) { + return readFileSync(join(projectDir, path), 'utf8') +} + +function sourceBetween(contents, startMarker, endMarker) { + const start = contents.indexOf(startMarker) + const end = contents.indexOf(endMarker, start + startMarker.length) + if (start === -1 || end === -1) { + throw new Error(`Missing source boundary: ${startMarker} → ${endMarker}`) + } + return contents.slice(start, end) +} + +describe('Windows computer-use horizontal scroll', () => { + it('routes left and right through the horizontal wheel with native signs', () => { + const windows = source('native/computer-use-windows/runtime.ps1') + const mouseEvents = sourceBetween(windows, '$MouseEvents = @{', 'function Write-OrcaJson') + const scroll = sourceBetween(windows, ' "scroll" {', ' "drag" {') + const left = sourceBetween( + scroll, + '} elseif ($Operation.direction -eq "left") {', + '} elseif ($Operation.direction -eq "right") {' + ) + const right = sourceBetween( + scroll, + '} elseif ($Operation.direction -eq "right") {', + '} elseif ($Operation.direction -ne "up") {' + ) + + expect(mouseEvents).toContain('HorizontalWheel = 0x01000') + expect(scroll).toContain('$mouseEvent = $MouseEvents.Wheel') + expect(left).toContain('$mouseEvent = $MouseEvents.HorizontalWheel') + expect(left).toContain('$delta = -1 * $delta') + expect(right).toContain('$mouseEvent = $MouseEvents.HorizontalWheel') + expect(right).not.toContain('$delta = -1 * $delta') + expect(scroll).toContain( + '[OrcaDesktopWin32]::mouse_event($mouseEvent, 0, 0, $delta, [UIntPtr]::Zero)' + ) + expect(scroll).not.toContain('mouse_event($MouseEvents.Wheel') + expect(scroll).toContain('throw "unsupported scroll direction: $($Operation.direction)"') + }) +}) diff --git a/native/computer-use-windows/runtime.ps1 b/native/computer-use-windows/runtime.ps1 index 2aa581901e6..4b68525c7c6 100644 --- a/native/computer-use-windows/runtime.ps1 +++ b/native/computer-use-windows/runtime.ps1 @@ -179,6 +179,7 @@ $MouseEvents = @{ MiddleDown = 0x0020 MiddleUp = 0x0040 Wheel = 0x0800 + HorizontalWheel = 0x01000 } function Write-OrcaJson($Payload) { @@ -1235,16 +1236,22 @@ function Invoke-OrcaOperation($Operation) { } "scroll" { $delta = 120 * [int][Math]::Ceiling((Get-OrcaPositiveNumber $Operation.pages "pages")) - if ($Operation.direction -eq "down" -or $Operation.direction -eq "right") { + $mouseEvent = $MouseEvents.Wheel + if ($Operation.direction -eq "down") { $delta = -1 * $delta - } elseif ($Operation.direction -ne "up" -and $Operation.direction -ne "left") { + } elseif ($Operation.direction -eq "left") { + $mouseEvent = $MouseEvents.HorizontalWheel + $delta = -1 * $delta + } elseif ($Operation.direction -eq "right") { + $mouseEvent = $MouseEvents.HorizontalWheel + } elseif ($Operation.direction -ne "up") { throw "unsupported scroll direction: $($Operation.direction)" } $point = Get-OrcaElementScreenPoint $element if ($null -eq $point) { $point = Get-OrcaScreenPoint $Operation $windowFrame } [void][OrcaDesktopWin32]::SetForegroundWindow($handle) [void][OrcaDesktopWin32]::SetCursorPos([int]$point.x, [int]$point.y) - [OrcaDesktopWin32]::mouse_event($MouseEvents.Wheel, 0, 0, $delta, [UIntPtr]::Zero) + [OrcaDesktopWin32]::mouse_event($mouseEvent, 0, 0, $delta, [UIntPtr]::Zero) $action = [pscustomobject]@{ path = "synthetic"; actionName = "scroll"; fallbackReason = $null } } "drag" { From 80b0ab16ff85aea4597cd216c36f8942998dfa04 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:07:33 -0700 Subject: [PATCH 8/8] test(crash-reporting): keep ambiguous whole-tree kills reportable (#14667) * test(crash-reporting): keep ambiguous tree kills reportable * test(crash-reporting): drop the stale sibling-settle deferral comment --- .../process-gone-killed-one-ordering.test.ts | 28 ++++++++++++++++++- .../process-gone-recorder.test.ts | 7 ++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts index 7f47e794252..72c897088f5 100644 --- a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts +++ b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts @@ -17,6 +17,9 @@ import { import { ProcessGoneDedupe } from './process-gone-dedupe' import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' +const noMinidump = async () => null +const attachDetails = async () => null + function event(overrides: Partial = {}): ProcessGoneCrashEvent { return { source: 'renderer', @@ -82,9 +85,32 @@ describe('recordProcessGoneCrash killed/1 ordering', () => { recordProcessGoneCrash({ record } as never, gpuKill, dedupe) recordProcessGoneCrash({ record } as never, networkServiceKill, dedupe) - recordProcessGoneCrash({ record } as never, rendererKill, dedupe) + recordProcessGoneCrash( + { record, attachDetails } as never, + rendererKill, + dedupe, + noMinidump + ) expect(record).toHaveBeenCalledOnce() + // Timing proximity is evidence, not authority to discard an ambiguous report. + expect(record).toHaveBeenCalledWith( + expect.objectContaining({ + breadcrumbs: expect.arrayContaining([ + expect.objectContaining({ + name: 'process_gone_suppressed', + data: expect.objectContaining({ source: 'child', processType: 'GPU' }) + }), + expect.objectContaining({ + name: 'process_gone_suppressed', + data: expect.objectContaining({ + source: 'child', + serviceName: 'network.mojom.NetworkService' + }) + }) + ]) + }) + ) }) it('suppresses the fleet sequence only after independent session-end intent', () => { diff --git a/src/main/crash-reporting/process-gone-recorder.test.ts b/src/main/crash-reporting/process-gone-recorder.test.ts index 8a0f77dd1e2..70e6ca93f04 100644 --- a/src/main/crash-reporting/process-gone-recorder.test.ts +++ b/src/main/crash-reporting/process-gone-recorder.test.ts @@ -436,10 +436,9 @@ describe('recordProcessGoneCrash', () => { } } - // Why child kills: the decode gate is source-agnostic, and a non-recoverable - // child persists synchronously on every branch of the crash-reporting stack - // (the renderer killed path defers behind a sibling-kill settle), so the - // platform stub is still in force when the gate reads process.platform. + // Why child kills: the decode gate is source-agnostic and reads + // process.platform synchronously at record time, so the platform stub is + // still in force when it runs. const nonRecoverableChildKill = (overrides: Partial): ProcessGoneCrashEvent => event({ source: 'child',