diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index a9566939925..1c352a1f7e5 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 27m + + downloads: 28m @@ -15,7 +15,7 @@ downloads downloads - 27m - 27m + 28m + 28m diff --git a/src/main/agent-hooks/ended-process-reconciliation.test.ts b/src/main/agent-hooks/ended-process-reconciliation.test.ts new file mode 100644 index 00000000000..0719b584d8a --- /dev/null +++ b/src/main/agent-hooks/ended-process-reconciliation.test.ts @@ -0,0 +1,179 @@ +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { makePaneKey } from '../../shared/stable-pane-id' +import { AgentHookServer } from './server' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const PANE = makePaneKey('tab-1', LEAF) + +let dir: string + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'orca-ended-process-')) +}) + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }) +}) + +async function startServer(): Promise { + const server = new AgentHookServer() + await server.start({ env: 'production', userDataPath: dir }) + return server +} + +function claudeRow(server: AgentHookServer, state: 'working' | 'waiting' | 'done'): void { + server.ingestTerminalStatus({ + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + connectionId: null, + payload: { state, prompt: 'review the PR', agentType: 'claude' } + }) +} + +/** A row carrying a resumable provider session — the only shape `dropStatusEntry` retains. */ +function resumableClaudeRow(server: AgentHookServer): void { + server.ingestRemote( + { + paneKey: PANE, + tabId: 'tab-1', + worktreeId: 'wt-1', + source: 'claude', + hookEventName: 'UserPromptSubmit', + launchToken: 'dead-launch-token', + providerSession: { key: 'session_id', id: 'resume-me' }, + payload: { state: 'working', prompt: 'review the PR', agentType: 'claude' } + }, + 'conn-a' + ) +} + +function paneState(server: AgentHookServer): string { + return server.getStatusSnapshotForPane(PANE)[0]?.state ?? 'missing' +} + +// The teardown that normally clears a dead pane depends on the spawn-time ptyPaneKey mapping, which +// a restored PTY may never rebuild — leaving the row and its latches with nothing left to retire +// them. See PLAN-STA-4612 §4.2. +describe('reconcileEndedProcessForPaneKeys', () => { + it('retires a working row whose process is certifiably dead', async () => { + const server = await startServer() + try { + claudeRow(server, 'working') + expect(paneState(server)).toBe('working') + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(paneState(server)).toBe('missing') + } finally { + server.stop() + } + }) + + it('retires a waiting row — a wait whose owner is dead is not a wait', async () => { + const server = await startServer() + try { + claudeRow(server, 'waiting') + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(paneState(server)).toBe('missing') + } finally { + server.stop() + } + }) + + it('clears Claude latches even when the stored row already reads done', async () => { + // An interrupted lead suppresses the gate while leaving the latch set, so a row can read `done` + // with a latch that would re-gate `working` on the pane's very next event. + const server = await startServer() + try { + claudeRow(server, 'done') + server._getStateForTests().claudeRunningNonAgentTaskPaneKeys.add(PANE) + server._getStateForTests().claudeActiveSessionCronPaneKeys.add(PANE) + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(server._getStateForTests().claudeRunningNonAgentTaskPaneKeys.has(PANE)).toBe(false) + expect(server._getStateForTests().claudeActiveSessionCronPaneKeys.has(PANE)).toBe(false) + } finally { + server.stop() + } + }) + + it('is a no-op for a pane with nothing to retire', async () => { + const server = await startServer() + try { + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(0) + } finally { + server.stop() + } + }) + + it('keeps the resume identity a paired dismissal minted when the shell outlived the agent', async () => { + // The renderer's confirmed-shell route calls agentStatus:drop FIRST, which deliberately mints a + // providerSessionOnly remnant, then this. That remnant carries no state claim — it cannot gate a + // pane `working` — and the pane's PTY is still there to resume into, so retiring the pane's live + // claims must not take it. Without the option it goes with everything else. + const server = await startServer() + try { + resumableClaudeRow(server) + server.dropStatusEntry(PANE) + const afterDrop = server.getStatusSnapshotForPane(PANE)[0] + expect(afterDrop?.providerSessionOnly).toBe(true) + expect(afterDrop?.providerSession?.id).toBe('resume-me') + + expect( + server.reconcileEndedProcessForPaneKeys([PANE], { preserveResumeIdentity: true }) + ).toBe(1) + + const kept = server.getStatusSnapshotForPane(PANE)[0] + expect(kept?.providerSessionOnly).toBe(true) + expect(kept?.providerSession?.id).toBe('resume-me') + expect(kept?.launchToken).toBeUndefined() + // The live claims still went: a latch left behind would re-gate the pane on its next event. + expect(server._getStateForTests().claudeRunningNonAgentTaskPaneKeys.has(PANE)).toBe(false) + + // The retained identity must not recreate dead launch authority after restart. + server.flushStatusPersistSync() + server.stop() + const restarted = await startServer() + expect(restarted.getStatusSnapshotForPane(PANE)[0]?.launchToken).toBeUndefined() + expect(restarted.getHydratedAuthorityCommitments()).toHaveLength(0) + restarted.stop() + } finally { + server.stop() + } + }) + + it('takes the resume identity too on a certified PTY exit, where no pane is left to resume into', async () => { + const server = await startServer() + try { + resumableClaudeRow(server) + server.dropStatusEntry(PANE) + + expect(server.reconcileEndedProcessForPaneKeys([PANE])).toBe(1) + + expect(paneState(server)).toBe('missing') + } finally { + server.stop() + } + }) + + it('drops the session owner so a rebound pane is not read as a session replacement', async () => { + const server = await startServer() + try { + claudeRow(server, 'working') + server._getStateForTests().claudeSessionOwnerByPaneKey.set(PANE, 'session-a') + + server.reconcileEndedProcessForPaneKeys([PANE]) + + expect(server._getStateForTests().claudeSessionOwnerByPaneKey.has(PANE)).toBe(false) + } finally { + server.stop() + } + }) +}) diff --git a/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts b/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts index 9811773eb07..b3257e6972c 100644 --- a/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts +++ b/src/main/agent-hooks/restored-subagent-liveness-sweep.test.ts @@ -211,7 +211,10 @@ describe('restored subagent liveness sweep', () => { } }) - it('does not probe a hydrated Claude pane without restored rows', async () => { + it('reaps a hydrated non-terminal row with no roster at all (STA-4612)', async () => { + // A restored roster is only one shape of stranded claim. A lead row left `working` with no + // children has nothing left to retire it — no child event, no inventory, no hook — so it holds + // the pane working for good and locks its agent out of hibernation. const server = await restartWithInFlightSubagent({ subagents: [] }) const probeLiveLocalPty = vi.fn(() => false) try { @@ -220,9 +223,42 @@ describe('restored subagent liveness sweep', () => { probeLiveLocalPty, persistedPtyIdByPaneKey: { [PANE]: PTY } }) + ).toBe(1) + + expect(probeLiveLocalPty).toHaveBeenCalled() + expect(paneStatus(server).state).toBe('missing') + } finally { + server.stop() + } + }) + + it('keeps a hydrated non-terminal row with no roster when its PTY is still live', async () => { + const server = await restartWithInFlightSubagent({ subagents: [] }) + try { + expect( + await sweepWith(server, { + probeLiveLocalPty: () => true, + persistedPtyIdByPaneKey: { [PANE]: PTY } + }) ).toBe(0) - expect(probeLiveLocalPty).not.toHaveBeenCalled() + expect(paneStatus(server).state).toBe('working') + } finally { + server.stop() + } + }) + + it('keeps a hydrated non-terminal row with no roster when the probe cannot prove either state', async () => { + const server = await restartWithInFlightSubagent({ subagents: [] }) + try { + expect( + await sweepWith(server, { + probeLiveLocalPty: () => null, + persistedPtyIdByPaneKey: { [PANE]: PTY } + }) + ).toBe(0) + + expect(paneStatus(server).state).toBe('working') } finally { server.stop() } diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 65b7b1479fd..9f39460e719 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -12,6 +12,7 @@ import { ORCA_HOOK_PROTOCOL_VERSION } from '../../shared/agent-hook-types' import { clearAllListenerCaches, clearPaneCacheState, + paneHasStateClaims, clearClaudeAnsweredQuestionWait, createHookListenerState, getEndpointFileName, @@ -2652,28 +2653,87 @@ export class AgentHookServer { this.notifyStatusChangeListeners() } + /** The resume-identity remnant of a dropped row: a `providerSessionOnly` entry carries no state + * claim — it cannot gate a pane `working` — so it survives teardowns that end the pane's live + * claims. Returns null when the row has no resumable session to keep. */ + private toRetainedProviderSessionRow( + entry: EnrichedAgentHookEventPayload | null | undefined + ): EnrichedAgentHookEventPayload | null { + if ( + !entry?.providerSession || + !entry.payload.agentType || + entry.payload.agentType === 'unknown' + ) { + return null + } + const { launchToken: _launchToken, ...resumeIdentity } = entry + return { ...resumeIdentity, providerSessionOnly: true, retainedForLiveness: true } + } + /** Drop only the status row (user dismissal); do NOT wipe prompt/tool caches since the pane's agent may still be alive. Use clearPaneState for PTY-teardown. */ dropStatusEntry(paneKey: string): void { const deleted = this.deleteStatusEntry(paneKey, { preserveAuthority: true }) if (!deleted) { return } - if ( - deleted.providerSession && - deleted.payload.agentType && - deleted.payload.agentType !== 'unknown' - ) { - const retained: EnrichedAgentHookEventPayload = { - ...deleted, - providerSessionOnly: true, - retainedForLiveness: true - } + const retained = this.toRetainedProviderSessionRow(deleted) + if (retained) { this.state.lastStatusByPaneKey.set(deleted.paneKey, retained) } this.scheduleStatusPersist() this.notifyStatusChangeListeners() } + /** Retire panes whose owning process is certifiably dead. + * + * The ordinary teardown already does this: every attributable PTY exit reaches + * `clearProviderPtyState`, which resolves the pane key and calls `clearPaneState`. But that + * resolution depends on the spawn-time `ptyPaneKey` mapping, which a restored/reattached PTY may + * never rebuild — so those panes keep a `working` row and its latches for good, with no hook left + * to retire them. This is the same operation reached from the runtime's own pane-key knowledge, + * so a dead pane is cleaned up identically however its keys were resolved. */ + reconcileEndedProcessForPaneKeys( + paneKeys: Iterable, + options?: { + /** The pane's PTY outlived its agent (a confirmed shell foreground), so the session can still + * be resumed in place — keep the `providerSessionOnly` remnant the paired `agentStatus:drop` + * minted for exactly this case. A certified PTY exit passes nothing: there is no pane left to + * resume into, and dropping it matches what `clearProviderPtyState` already does. */ + preserveResumeIdentity?: boolean + } + ): number { + let cleared = 0 + for (const paneKey of paneKeys) { + const resolvedPaneKey = this.resolvePaneKeyAlias(paneKey) + if (!this.hasLiveClaimsForPaneKey(resolvedPaneKey)) { + continue + } + const retained = options?.preserveResumeIdentity + ? this.toRetainedProviderSessionRow( + this.state.lastStatusByPaneKey.get(resolvedPaneKey) as + | EnrichedAgentHookEventPayload + | undefined + ) + : null + this.clearPaneState(resolvedPaneKey) + if (retained) { + this.state.lastStatusByPaneKey.set(resolvedPaneKey, retained) + this.scheduleStatusPersist() + this.notifyStatusChangeListeners() + } + cleared += 1 + } + return cleared + } + + /** Anything a dead pane could still be asserting: a row, or a latch that would re-gate one through + * `resolveClaudePaneState` on the pane's next event even after the row reads `done`. The list + * itself lives beside `clearPaneCacheState`, so adding a latch cannot leave this behind in a + * different file. */ + private hasLiveClaimsForPaneKey(paneKey: string): boolean { + return paneHasStateClaims(this.state, paneKey) + } + /** Clear statuses proven to belong to one lost SSH transport. */ clearStatusEntriesForConnection(connectionId: string): void { const normalizedConnectionId = connectionId.trim() @@ -2704,6 +2764,7 @@ export class AgentHookServer { this.state.claudeLeadStateByPaneKey.delete(paneKey) this.state.claudeRunningNonAgentTaskPaneKeys.delete(paneKey) this.state.claudeActiveSessionCronPaneKeys.delete(paneKey) + this.state.claudeSessionOwnerByPaneKey.delete(paneKey) } } } @@ -2894,9 +2955,15 @@ export class AgentHookServer { enriched.payload.agentType === 'claude' && enriched.connectionId === null && isLocalExecutionHost(enriched.worktreeId) && - claudeRosterHasRestoredSnapshotSubagent( + // Why: a restored roster is only one shape of stranded claim. A lead row left non-terminal, + // or a background-task/cron latch nothing will refresh, strands the pane just as + // permanently — and unlike the roster case there is no child event left to reap it. + (claudeRosterHasRestoredSnapshotSubagent( this.state.claudeSubagentRosterByPaneKey.get(paneKey) - ) && + ) || + enriched.payload.state !== 'done' || + this.state.claudeRunningNonAgentTaskPaneKeys.has(paneKey) || + this.state.claudeActiveSessionCronPaneKeys.has(paneKey)) && !this.runtimeObservedStatusPaneKeys.has(paneKey) ) { candidates.push({ paneKey, entry: enriched }) @@ -2924,6 +2991,20 @@ export class AgentHookServer { continue } if (!reapRestoredClaudeSubagentsForDeadPane(this.state, paneKey)) { + // Why: the roster reap only speaks for restored child rows. A pane whose PTY is provably + // gone and whose claim is a lead row or a latch has nothing for it to reap, so retire the + // pane the same way an observed exit would — otherwise the widened candidate set is inert. + // + // Why delete rather than downgrade to `done` like the reap branch below: that branch has a + // real turn to describe — a parent whose children it just reaped — while these panes' only + // claim IS the stale non-terminal row. Rewriting a `waiting`/`blocked` row to `done` would + // invent a completion that never happened, and leaving it non-terminal keeps the bug. This + // sweep stands in for the exit Orca never observed, so it does what that exit does: + // `clearProviderPtyState` -> `clearPaneState`. + if (this.hasLiveClaimsForPaneKey(paneKey)) { + this.clearPaneState(paneKey) + changedPanes += 1 + } continue } changedPanes += 1 diff --git a/src/main/codex/codex-hooks-read-denial-followup.test.ts b/src/main/codex/codex-hooks-read-denial-followup.test.ts new file mode 100644 index 00000000000..1efb40f571d --- /dev/null +++ b/src/main/codex/codex-hooks-read-denial-followup.test.ts @@ -0,0 +1,151 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as NodeFs from 'node:fs' +import type * as NodeOs from 'node:os' +import type * as CodexManagedTrustReconciliation from './codex-managed-trust-reconciliation' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +// Follow-up to STA-4823: two more reads on the Codex hook paths acted on +// "absent" when the real answer was "could not read". Both were found while +// reviewing #15417 and are not covered by it. +// +// The denial shape here is the MEASURED one: chmod 000 / icacls deny leave +// existsSync true and stat succeeding, and fail only the content read +// (EACCES -13 on macOS, EPERM -4048 on Windows). No mode fakes existsSync, +// because no platform does. + +const denials = vi.hoisted(() => { + const state = { + paths: new Set(), + deny(path: string): void { + state.paths.add(path) + }, + reset(): void { + state.paths.clear() + }, + check(target: unknown, syscall: string): void { + if (typeof target !== 'string' || !state.paths.has(target)) { + return + } + const error: NodeJS.ErrnoException = new Error( + `EACCES: permission denied, ${syscall} '${target}'` + ) + error.code = 'EACCES' + error.errno = -13 + error.syscall = syscall + error.path = target + throw error + } + } + return state +}) + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal() + const guardRead = (fn: unknown, syscall: string): unknown => { + const original = fn as (...args: unknown[]) => unknown + const wrapped = (...args: unknown[]): unknown => { + denials.check(args[0], syscall) + return original(...args) + } + return Object.assign(wrapped, original) + } + const patched: Record = { + ...actual, + readFileSync: guardRead(actual.readFileSync, 'read'), + // Why: existsSync and stat stay HEALTHY. That is the whole point — a + // permission denial does not hide the file, it only refuses its contents. + openSync: Object.assign((...args: unknown[]): unknown => { + const flags = args[1] + if (flags === undefined || (typeof flags === 'string' && flags.startsWith('r'))) { + denials.check(args[0], 'open') + } + return (actual.openSync as (...a: unknown[]) => unknown)(...args) + }, actual.openSync) + } + return { ...patched, default: patched } +}) + +const { getPathMock, homedirMock } = vi.hoisted(() => ({ + getPathMock: vi.fn<(name: string) => string>(), + homedirMock: vi.fn<() => string>() +})) + +vi.mock('electron', () => ({ app: { getPath: getPathMock } })) +vi.mock('node:os', async () => { + const actual = await vi.importActual('node:os') + return { ...actual, homedir: homedirMock } +}) + +const realFs = await vi.importActual('node:fs') + +let fakeHomeDir: string +let userDataDir: string +let previousUserDataPath: string | undefined + +beforeEach(() => { + denials.reset() + fakeHomeDir = realFs.mkdtempSync(join(tmpdir(), 'orca-hooksread-home-')) + userDataDir = realFs.mkdtempSync(join(tmpdir(), 'orca-hooksread-data-')) + previousUserDataPath = process.env.ORCA_USER_DATA_PATH + process.env.ORCA_USER_DATA_PATH = userDataDir + homedirMock.mockReturnValue(fakeHomeDir) + getPathMock.mockImplementation((name: string) => { + if (name === 'userData') { + return userDataDir + } + throw new Error(`unexpected app.getPath(${name})`) + }) + realFs.mkdirSync(join(fakeHomeDir, '.codex'), { recursive: true }) +}) + +afterEach(() => { + denials.reset() + realFs.rmSync(fakeHomeDir, { recursive: true, force: true }) + realFs.rmSync(userDataDir, { recursive: true, force: true }) + if (previousUserDataPath === undefined) { + delete process.env.ORCA_USER_DATA_PATH + } else { + process.env.ORCA_USER_DATA_PATH = previousUserDataPath + } + vi.clearAllMocks() +}) + +describe('an unreadable legacy hooks.json must not clear managed trust or its ledger', () => { + it('returns before the removal branch instead of discarding approvals', async () => { + const legacyHooksPath = join(fakeHomeDir, '.codex', 'hooks.json') + realFs.writeFileSync(legacyHooksPath, JSON.stringify({ hooks: { Stop: [] } }), 'utf-8') + + const removeSpy = vi.fn() + vi.resetModules() + vi.doMock('./codex-managed-trust-reconciliation', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, removeCodexManagedHookTrustEntries: removeSpy } + }) + try { + // The removal only fires when a real-home grant is on record — without + // this the branch is inert and the test proves nothing either way. + const ledger = await import('./codex-trust-grant-ledger') + ledger.writeCodexTrustGrantLedgerHome( + join(fakeHomeDir, '.codex'), + { entries: { 'seed:1': { trustedHash: 'sha256:seed' } } } as never, + ledger.getCodexTrustGrantLedgerPath() + ) + + const hookService = await import('./hook-service') + denials.deny(legacyHooksPath) + + hookService._internals.cleanupLegacySystemManagedHooks() + + // `readHooksJsonWithRaw` catches the denial and reports {raw:null, + // config:null} — "could not read". Before the fix that fell into the same + // branch as "no hooks configured", which removes the managed trust + // entries AND their grant-ledger record. A read that failed is not + // permission to discard approvals the user already gave. + expect(removeSpy).not.toHaveBeenCalled() + } finally { + vi.doUnmock('./codex-managed-trust-reconciliation') + vi.resetModules() + } + }) +}) diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index a0e30c5941c..270906012b5 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -601,6 +601,13 @@ function cleanupLegacySystemManagedHooks(): void { // Why: the pre-write guard below compares against these bytes; a separate // later read would let a concurrent save land between parse and snapshot. const { raw: previousRaw, config } = readHooksJsonWithRaw(legacyConfigPath) + // Why: `config === null` with no raw is the "could not read" answer, not the + // "no hooks here" one — the branch below removes managed trust entries AND + // their grant-ledger record, so acting on it would discard approvals over a + // read that merely failed. A genuine absence still returns `config: {}`. + if (config === null && previousRaw === null) { + return + } if (!config?.hooks || previousRaw === null) { if (hasRecordedRealHomeGrant) { removeSystemManagedHookTrustEntries(systemHomePath, legacyConfigPath) @@ -1637,6 +1644,7 @@ export class CodexHookService { export const codexHookService = new CodexHookService() export const _internals = { + cleanupLegacySystemManagedHooks, getManagedScript, installManagedHooksIntoWslRuntime, refreshWslRuntimeUserHooks, diff --git a/src/main/index.ts b/src/main/index.ts index ae8fed9981d..da9c334c7e1 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2719,6 +2719,9 @@ void app.whenReady().then(async () => { agentHookServer.attestCompatibilityAuthority(candidate), retireAgentHookCompatibilityAuthority: (paneKey) => agentHookServer.retirePaneAuthority(paneKey), + reconcileAgentStatusForEndedProcess: (paneKeys) => { + agentHookServer.reconcileEndedProcessForPaneKeys(paneKeys) + }, canRecoverPersistentLocalPtys: () => getDaemonProvider() !== null, // Why: source codex-home here (runs in window AND serve) so aiVault.listSessions includes managed-Codex sessions; registerCoreHandlers is window-only. getAdditionalAiVaultCodexHomePaths: () => diff --git a/src/main/ipc/agent-hooks.ts b/src/main/ipc/agent-hooks.ts index 5a8e3f69ae4..5615fa38208 100644 --- a/src/main/ipc/agent-hooks.ts +++ b/src/main/ipc/agent-hooks.ts @@ -6,13 +6,9 @@ import type { } from '../../shared/agent-status-types' import type { AgentInterruptInferenceRequest } from '../../shared/agent-interrupt-intent' import type { AgentQuestionAnsweredInferenceRequest } from '../../shared/agent-question-answered-intent' -import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' +import { agentHookServer } from '../agent-hooks/server' import { ampHookService } from '../amp/hook-service' -import { - clearMigrationUnsupportedPtysByTabPrefix, - clearMigrationUnsupportedPtysForPaneKey, - getMigrationUnsupportedPtySnapshot -} from '../agent-hooks/migration-unsupported-pty-state' +import { getMigrationUnsupportedPtySnapshot } from '../agent-hooks/migration-unsupported-pty-state' import { claudeHookService } from '../claude/hook-service' import { codexHookService } from '../codex/hook-service' import { geminiHookService } from '../gemini/hook-service' @@ -27,10 +23,10 @@ import { devinHookService } from '../devin/hook-service' import { kimiHookService } from '../kimi/hook-service' import { openClaudeHookService } from '../openclaude/hook-service' import { registerAgentPaneAuthorityIpcHandlers } from './agent-pane-authority-ipc' +import { registerAgentStatusRowTeardownIpcHandlers } from './agent-status-row-teardown-ipc' import { createAgentPaneAuthorityOwnership } from './agent-pane-authority-ownership' import { enrichAgentStatusIpcPayload, - isValidAgentStatusDropTabId, type AgentStatusRuntimeEnrichment } from './agent-status-ipc-boundary' @@ -70,38 +66,7 @@ export function registerAgentHookHandlers( ipcMain.removeHandler('agentStatus:inferInterrupt') ipcMain.removeHandler('agentStatus:inferQuestionAnswered') ipcMain.removeHandler('agentStatus:getMigrationUnsupportedSnapshot') - // Why: agentStatus:drop is sent fire-and-forget from the renderer via - // ipcRenderer.send(); we listen with ipcMain.on (not handle) so we don't - // round-trip a response. Removing first keeps re-registration safe even - // though the module-level registered guard already prevents re-entry today. - ipcMain.removeAllListeners('agentStatus:drop') - ipcMain.removeAllListeners('agentStatus:dropByTabPrefix') - ipcMain.on('agentStatus:drop', (_event, paneKey: unknown) => { - if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { - return - } - try { - // Why: dropStatusEntry (not clearPaneState) is correct here — the user is - // dismissing a status row, not tearing down a PTY. clearPaneState would also - // wipe the per-pane prompt/tool caches, which the next hook event for that - // (still-alive) pane needs to render a coherent row. - agentHookServer.dropStatusEntry(paneKey) - clearMigrationUnsupportedPtysForPaneKey(paneKey) - } catch (err) { - console.warn('[agent-hooks] dropStatusEntry failed:', err) - } - }) - ipcMain.on('agentStatus:dropByTabPrefix', (_event, tabId: unknown) => { - if (!isValidAgentStatusDropTabId(tabId)) { - return - } - try { - agentHookServer.dropStatusEntriesByTabPrefix(tabId) - clearMigrationUnsupportedPtysByTabPrefix(tabId) - } catch (err) { - console.warn('[agent-hooks] dropStatusEntriesByTabPrefix failed:', err) - } - }) + registerAgentStatusRowTeardownIpcHandlers() registerAgentPaneAuthorityIpcHandlers({ ownsPty: createAgentPaneAuthorityOwnership({ getPtyIdForPaneKey: dependencies.getPtyIdForPaneKey, diff --git a/src/main/ipc/agent-status-row-teardown-ipc.ts b/src/main/ipc/agent-status-row-teardown-ipc.ts new file mode 100644 index 00000000000..e33e1c93789 --- /dev/null +++ b/src/main/ipc/agent-status-row-teardown-ipc.ts @@ -0,0 +1,69 @@ +import { ipcMain } from 'electron' +import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' +import { + clearMigrationUnsupportedPtysByTabPrefix, + clearMigrationUnsupportedPtysForPaneKey +} from '../agent-hooks/migration-unsupported-pty-state' +import { isValidAgentStatusDropTabId } from './agent-status-ipc-boundary' + +/** + * The three renderer-initiated ways a status row goes away. All fire-and-forget + * (`ipcRenderer.send` → `ipcMain.on`), so none round-trips a response; removing the + * listeners first keeps re-registration safe. + * + * They are NOT interchangeable. A dismissal keeps the pane's per-pane caches because its agent may + * still be alive; a confirmed process exit must take them too, or a surviving Claude latch resolves + * the pane's next event straight back to `working`. + */ +export function registerAgentStatusRowTeardownIpcHandlers(): void { + ipcMain.removeAllListeners('agentStatus:drop') + ipcMain.removeAllListeners('agentStatus:reconcileEndedProcess') + ipcMain.removeAllListeners('agentStatus:dropByTabPrefix') + + ipcMain.on('agentStatus:drop', (_event, paneKey: unknown) => { + if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { + return + } + try { + // Why: dropStatusEntry (not clearPaneState) is correct here — the user is + // dismissing a status row, not tearing down a PTY. clearPaneState would also + // wipe the per-pane prompt/tool caches, which the next hook event for that + // (still-alive) pane needs to render a coherent row. + agentHookServer.dropStatusEntry(paneKey) + clearMigrationUnsupportedPtysForPaneKey(paneKey) + } catch (err) { + console.warn('[agent-hooks] dropStatusEntry failed:', err) + } + }) + + ipcMain.on('agentStatus:reconcileEndedProcess', (_event, paneKey: unknown) => { + if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { + return + } + try { + // Why: a process-table-confirmed agent exit is exactly the case the dismissal above excludes + // — the pane's agent is NOT still alive — so its latches must go with the row (STA-4612). + agentHookServer.reconcileEndedProcessForPaneKeys([paneKey], { + // Why: this route only fires on a confirmed shell foreground, so the PTY outlived the + // agent. The row's resume identity is still usable in that very pane — only its live + // claims are dead. + preserveResumeIdentity: true + }) + clearMigrationUnsupportedPtysForPaneKey(paneKey) + } catch (err) { + console.warn('[agent-hooks] reconcileEndedProcessForPaneKeys failed:', err) + } + }) + + ipcMain.on('agentStatus:dropByTabPrefix', (_event, tabId: unknown) => { + if (!isValidAgentStatusDropTabId(tabId)) { + return + } + try { + agentHookServer.dropStatusEntriesByTabPrefix(tabId) + clearMigrationUnsupportedPtysByTabPrefix(tabId) + } catch (err) { + console.warn('[agent-hooks] dropStatusEntriesByTabPrefix failed:', err) + } + }) +} diff --git a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts index ae4de312c55..c99c7f7b93d 100644 --- a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts +++ b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts @@ -289,7 +289,9 @@ describe('registerPtyHandlers', () => { await Promise.resolve() expect(runtime.onPtyExit).toHaveBeenCalledTimes(1) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, { + providerExitObserved: true + }) expect( mainWindow.webContents.send.mock.calls.filter((call) => call[0] === 'pty:exit') ).toEqual([['pty:exit', { id: 'local-pty', code: 0 }]]) @@ -342,7 +344,9 @@ describe('registerPtyHandlers', () => { await expect(stopPromise).resolves.toBe(true) expect(runtime.onPtyExit).toHaveBeenCalledTimes(1) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, { + providerExitObserved: true + }) expect( mainWindow.webContents.send.mock.calls.filter((call) => call[0] === 'pty:exit') ).toEqual([['pty:exit', { id: 'local-pty', code: 0 }]]) diff --git a/src/main/ipc/pty-listener-teardown-and-orphans.test.ts b/src/main/ipc/pty-listener-teardown-and-orphans.test.ts index 46b15d86bcb..ae749f8dbe0 100644 --- a/src/main/ipc/pty-listener-teardown-and-orphans.test.ts +++ b/src/main/ipc/pty-listener-teardown-and-orphans.test.ts @@ -440,6 +440,7 @@ describe('registerPtyHandlers', () => { expect(killSpy).toHaveBeenCalled() expect(runtime.onPtyExit).toHaveBeenCalledWith(spawnResult.id, -1, spawnResult.incarnationId, { + providerExitObserved: true, cause: { kind: 'unknown', reason: 'stop_unverified' } }) const listed = await getLocalPtyProvider().listProcesses() diff --git a/src/main/ipc/pty-runtime-kill-and-exit.test.ts b/src/main/ipc/pty-runtime-kill-and-exit.test.ts index 478212b6c15..2200b2b6043 100644 --- a/src/main/ipc/pty-runtime-kill-and-exit.test.ts +++ b/src/main/ipc/pty-runtime-kill-and-exit.test.ts @@ -287,7 +287,9 @@ describe('registerPtyHandlers', () => { await handlers.get('pty:kill')!(null, { id: 'local-pty' }) expect(runtime.onPtyExit).toHaveBeenCalledTimes(1) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-pty', 0, undefined, { + providerExitObserved: true + }) expect(mainWindow.webContents.send.mock.calls.filter((call) => call[0] === 'pty:exit')).toEqual( [['pty:exit', { id: 'local-pty', code: 0 }]] ) @@ -529,7 +531,9 @@ describe('registerPtyHandlers', () => { seq: 13, rawLength: 'daemon output'.length }) - expect(runtime.onPtyExit).toHaveBeenCalledWith(result.id, 0, undefined, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith(result.id, 0, undefined, { + providerExitObserved: true + }) expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:exit', { id: result.id, code: 0 diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index cac4538be37..3adcdb56376 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -2575,7 +2575,12 @@ export function registerPtyHandlers( clearProviderPtyState(id) ptyOwnership.delete(id) markClaudePtyExited(id) - runtime?.onPtyExit(id, code, incarnationId, cause ? { cause } : undefined) + // Why: the provider's physical exit callback is a death certificate even when node-pty + // reports the code as -1; only the synthesized failed/unverified stop calls omit this. + runtime?.onPtyExit(id, code, incarnationId, { + providerExitObserved: true, + ...(cause ? { cause } : {}) + }) }, onData: (id, data, timestamp, sequenceChars, transformed) => runtime?.onPtyData(id, data, timestamp, sequenceChars ?? data.length, transformed) @@ -4116,12 +4121,12 @@ export function registerPtyHandlers( clearProviderPtyState(payload.id) ptyOwnership.delete(payload.id) markClaudePtyExited(payload.id) - runtime?.onPtyExit( - payload.id, - payload.code, - payload.incarnationId, - payload.cause ? { cause: payload.cause } : undefined - ) + // Why: same physical-exit certificate as the LocalPtyProvider branch above — this is the + // daemon/WSL callback, and on macOS it is the branch that actually runs. + runtime?.onPtyExit(payload.id, payload.code, payload.incarnationId, { + providerExitObserved: true, + ...(payload.cause ? { cause: payload.cause } : {}) + }) } // Why not the whole payload: the exit cause is a main-process fact for the // runtime's records; the renderer's pty:exit contract stays as it was. diff --git a/src/main/ipc/remote-workspace.ts b/src/main/ipc/remote-workspace.ts index 527a001bb67..2b8966c3297 100644 --- a/src/main/ipc/remote-workspace.ts +++ b/src/main/ipc/remote-workspace.ts @@ -9,6 +9,7 @@ import type { } from '../../shared/remote-workspace-types' import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' import { getRepoIdFromWorktreeId } from '../../shared/worktree/id' +import { parseExecutionHostId } from '../../shared/execution-host' import { getRemoteWorkspaceNamespace } from './remote-workspace-namespace' import { registerRemoteWorkspaceNotificationHandler } from './remote-workspace-events' import { CLIENT_ID } from './remote-workspace-client-identity' @@ -55,7 +56,15 @@ function getExplicitHydratedTargetIds(value: unknown): Set | null { return new Set(value) } -function targetForWorktree(store: Store, worktreeId: string): string | null { +function targetForWorktree( + store: Store, + worktreeId: string, + executionHostId?: string +): string | null { + const parsedHostId = parseExecutionHostId(executionHostId) + if (parsedHostId?.kind === 'ssh') { + return parsedHostId.targetId + } const repoId = getRepoIdFromWorktreeId(worktreeId) return store.getRepo(repoId)?.connectionId ?? null } @@ -66,7 +75,8 @@ function exportSessionForTarget( session: WorkspaceSessionState ): RemoteWorkspaceSession { return exportRemoteWorkspaceSession(session, { - isTargetWorktree: (worktreeId) => targetForWorktree(store, worktreeId) === targetId + isTargetWorktree: (worktreeId, executionHostId) => + targetForWorktree(store, worktreeId, executionHostId) === targetId }) } diff --git a/src/main/orca-profiles/profile-project-session-transfer.ts b/src/main/orca-profiles/profile-project-session-transfer.ts index d5884096bd9..da55794b551 100644 --- a/src/main/orca-profiles/profile-project-session-transfer.ts +++ b/src/main/orca-profiles/profile-project-session-transfer.ts @@ -39,6 +39,7 @@ function hasTransferredSessionState(session: WorkspaceSessionState): boolean { Object.keys(session.browserTabsByWorktree ?? {}).length > 0 || Object.keys(session.unifiedTabs ?? {}).length > 0 || Object.keys(session.tabGroups ?? {}).length > 0 || + Object.keys(session.lastVisitedAtByWorktreeId ?? {}).length > 0 || Object.keys(session.terminalTopologyRevisionByRepoId ?? {}).length > 0 ) } diff --git a/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts b/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts index 51d87cfc668..485861d748d 100644 --- a/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts +++ b/src/main/orca-profiles/profile-project-transfer-worktree-ids.ts @@ -2,6 +2,10 @@ import type { PersistedState } from '../../shared/persisted-state-types' import { parseWorkspaceKey } from '../../shared/workspace-scope' import type { TransferProfileState } from './profile-project-state-file' import { isRepoWorktreeId } from './profile-project-worktree-identity' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../shared/worktree/host-qualified-identity' export function collectTransferWorktreeIds( state: TransferProfileState, @@ -51,8 +55,9 @@ function collectSessionWorktreeIds( } const addOwnerKeys = (record: Record | undefined): void => { for (const key of Object.keys(record ?? {})) { - if (isRepoWorktreeId(repoId, key)) { - ids.add(key) + const rawKey = isWorktreeHostIdentity(key) ? getWorktreeIdFromHostIdentity(key) : key + if (isRepoWorktreeId(repoId, rawKey)) { + ids.add(rawKey) } const parsed = parseWorkspaceKey(key) if (parsed?.type === 'worktree' && isRepoWorktreeId(repoId, parsed.worktreeId)) { diff --git a/src/main/orca-profiles/profile-project-worktree-identity.ts b/src/main/orca-profiles/profile-project-worktree-identity.ts index cdc8d9182d2..1586a0e0120 100644 --- a/src/main/orca-profiles/profile-project-worktree-identity.ts +++ b/src/main/orca-profiles/profile-project-worktree-identity.ts @@ -4,6 +4,10 @@ import type { WorkspaceKey } from '../../shared/folder-workspace-types' import type { Repo } from '../../shared/repo-types' import { parseWorkspaceKey, worktreeWorkspaceKey } from '../../shared/workspace-scope' import { WORKTREE_ID_SEPARATOR } from '../../shared/worktree/id' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../shared/worktree/host-qualified-identity' export function repoPhysicalKey( repo: Pick @@ -46,8 +50,14 @@ export function rekeyOwnerKey( newRepoId: string, ownerKey: string ): string | null { - if (isRepoWorktreeId(oldRepoId, ownerKey)) { - return rekeyWorktreeId(oldRepoId, newRepoId, ownerKey) + const rawOwnerKey = isWorktreeHostIdentity(ownerKey) + ? getWorktreeIdFromHostIdentity(ownerKey) + : ownerKey + if (isRepoWorktreeId(oldRepoId, rawOwnerKey)) { + const rekeyed = rekeyWorktreeId(oldRepoId, newRepoId, rawOwnerKey) + return isWorktreeHostIdentity(ownerKey) + ? `${ownerKey.slice(0, ownerKey.length - rawOwnerKey.length)}${rekeyed}` + : rekeyed } const parsed = parseWorkspaceKey(ownerKey) if (parsed?.type === 'worktree' && isRepoWorktreeId(oldRepoId, parsed.worktreeId)) { @@ -57,7 +67,10 @@ export function rekeyOwnerKey( } export function ownerKeyBelongsToRepo(ownerKey: string, repoId: string): boolean { - if (isRepoWorktreeId(repoId, ownerKey)) { + const rawOwnerKey = isWorktreeHostIdentity(ownerKey) + ? getWorktreeIdFromHostIdentity(ownerKey) + : ownerKey + if (isRepoWorktreeId(repoId, rawOwnerKey)) { return true } const parsed = parseWorkspaceKey(ownerKey) diff --git a/src/main/persistence-cohort-and-identity-migration.test.ts b/src/main/persistence-cohort-and-identity-migration.test.ts index 9a8edc4a964..9d931a1bbd4 100644 --- a/src/main/persistence-cohort-and-identity-migration.test.ts +++ b/src/main/persistence-cohort-and-identity-migration.test.ts @@ -387,6 +387,19 @@ describe('Store.migrateWorktreeIdentity', () => { expect(store.getWorktreeMeta(NEWER)?.priorWorktreeIds).toEqual([OLD, NEW]) }) + it('keeps the newest visit timestamp when a partial migration left both identities', async () => { + const store = await createStore() + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + lastVisitedAtByWorktreeId: { [OLD]: 100, [NEW]: 900 } + }) + + store.migrateWorktreeIdentity(OLD, NEW) + + expect(store.getWorkspaceSession().lastVisitedAtByWorktreeId?.[OLD]).toBeUndefined() + expect(store.getWorkspaceSession().lastVisitedAtByWorktreeId?.[NEW]).toBe(900) + }) + it('is a no-op when the ids match', async () => { const store = await createStore() store.setWorktreeMeta(OLD, { displayName: 'Cunner' }) diff --git a/src/main/persistence-repo-lifecycle.test.ts b/src/main/persistence-repo-lifecycle.test.ts index 3266e92bb7f..e27287c147e 100644 --- a/src/main/persistence-repo-lifecycle.test.ts +++ b/src/main/persistence-repo-lifecycle.test.ts @@ -467,6 +467,32 @@ describe('Store', () => { ).toBe(222) }) + it('removeProjectForHost on local prunes the unqualified host-identity visit key', async () => { + const store = await createStore() + store.addRepo(makeRepo({ id: 'shared', path: '/repo' })) + store.addRepo( + makeRepo({ + id: 'shared', + path: '/repo', + connectionId: 'ssh-a', + executionHostId: 'ssh:ssh-a' + }) + ) + // No meta for this worktree, so only the unqualified host-identity key names it. The host split + // parks that canonical unknown-host form in the local partition; leaving it behind after a local + // removal re-materializes an orphaned workspace next launch. + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + lastVisitedAtByWorktreeId: { '|shared::/repo/wt-orphan': 111 } + }) + + store.removeProjectForHost('shared', 'local') + + expect( + store.getWorkspaceSession().lastVisitedAtByWorktreeId?.['|shared::/repo/wt-orphan'] + ).toBeUndefined() + }) + it('removeProjectForHost prunes only the removed host when a third host also shares the owner key', async () => { const store = await createStore() // Same repo id + path on local and two SSH hosts, so the owner key diff --git a/src/main/persistence/restoring-sessions/session-owner-fields.ts b/src/main/persistence/restoring-sessions/session-owner-fields.ts index ee06ada964b..c18d50ae612 100644 --- a/src/main/persistence/restoring-sessions/session-owner-fields.ts +++ b/src/main/persistence/restoring-sessions/session-owner-fields.ts @@ -1,6 +1,10 @@ import type { TerminalTab } from '../../../shared/terminal-tab-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' export function createMinimalPersistedTerminalTab(args: { worktreeId: string @@ -93,7 +97,12 @@ export function deleteOwnerKeyedSessionFields( delete next.activeGroupIdByWorktree[ownerKey] } if (next.lastVisitedAtByWorktreeId) { - delete next.lastVisitedAtByWorktreeId[ownerKey] + for (const key of Object.keys(next.lastVisitedAtByWorktreeId)) { + const rawId = isWorktreeHostIdentity(key) ? getWorktreeIdFromHostIdentity(key) : key + if (key === ownerKey || rawId === ownerKey) { + delete next.lastVisitedAtByWorktreeId[key] + } + } } if (next.defaultTerminalTabsAppliedByWorktreeId) { delete next.defaultTerminalTabsAppliedByWorktreeId[ownerKey] diff --git a/src/main/persistence/tracking-repos/repo-worktree-pruning.ts b/src/main/persistence/tracking-repos/repo-worktree-pruning.ts index f12da044c15..50c4fe5f745 100644 --- a/src/main/persistence/tracking-repos/repo-worktree-pruning.ts +++ b/src/main/persistence/tracking-repos/repo-worktree-pruning.ts @@ -3,6 +3,10 @@ import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/e import { parseWorkspaceKey } from '../../../shared/workspace-scope' import type { StoreOwnedPersistedState } from '../loading-store/store-owned-state' import { removeWorkspaceSessionOwners } from '../restoring-sessions/session-owner-removal' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' export function pruneWorktreeStateForRepo( state: StoreOwnedPersistedState, @@ -36,7 +40,18 @@ export function pruneWorktreeStateForRepo( const ownerKeysToPrune = new Set() const collectPrefixedKeys = (keys: Iterable): void => { for (const key of keys) { - if (key.startsWith(prefix)) { + const rawKey = isWorktreeHostIdentity(key) ? getWorktreeIdFromHostIdentity(key) : key + const keyHost = isWorktreeHostIdentity(key) ? key.slice(0, key.indexOf('|')) : null + // Why: bare keys are scoped by the per-partition gating below; only host-qualified + // keys can sit in another host's partition and need host-matching at collection time. + // Why the empty-host arm: the host split parks the canonical unknown-host form (`|`) in + // the local partition, so a local prune must claim it or the orphan restores next launch. + const belongsToPrunedHost = + hostId === null || + keyHost === null || + keyHost === hostId || + (keyHost === '' && hostId === LOCAL_EXECUTION_HOST_ID) + if (belongsToPrunedHost && rawKey.startsWith(prefix)) { ownerKeysToPrune.add(key) } } diff --git a/src/main/persistence/tracking-repos/worktree-identity-migration.ts b/src/main/persistence/tracking-repos/worktree-identity-migration.ts index f8256c0c4fe..a9d3397aa7c 100644 --- a/src/main/persistence/tracking-repos/worktree-identity-migration.ts +++ b/src/main/persistence/tracking-repos/worktree-identity-migration.ts @@ -1,6 +1,10 @@ import type { WorkspaceKey } from '../../../shared/folder-workspace-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' import type { StoreOwnedPersistedState } from '../loading-store/store-owned-state' /** @@ -94,7 +98,29 @@ export function migrateWorktreeIdentity( moveSessionKey(session.tabGroups, (groups) => groups.map(withNewWorktreeId)) || sessionChanged sessionChanged = moveSessionKey(session.tabGroupLayouts) || sessionChanged sessionChanged = moveSessionKey(session.activeGroupIdByWorktree) || sessionChanged - sessionChanged = moveSessionKey(session.lastVisitedAtByWorktreeId) || sessionChanged + if (session.lastVisitedAtByWorktreeId) { + const nextRecency = { ...session.lastVisitedAtByWorktreeId } + let recencyChanged = false + for (const [key, value] of Object.entries(session.lastVisitedAtByWorktreeId)) { + const rawId = isWorktreeHostIdentity(key) ? getWorktreeIdFromHostIdentity(key) : key + if (rawId !== oldWorktreeId) { + continue + } + const nextKey = isWorktreeHostIdentity(key) + ? `${key.slice(0, key.length - rawId.length)}${newWorktreeId}` + : newWorktreeId + // Why max: a partial migration leaves both identities behind; taking the older one would + // regress Cmd+J recency after restart. + const existing = nextRecency[nextKey] + nextRecency[nextKey] = existing === undefined ? value : Math.max(existing, value) + delete nextRecency[key] + recencyChanged = true + } + if (recencyChanged) { + session.lastVisitedAtByWorktreeId = nextRecency + sessionChanged = true + } + } sessionChanged = moveSessionKey(session.defaultTerminalTabsAppliedByWorktreeId) || sessionChanged if (session.activeWorktreeIdsOnShutdown?.includes(oldWorktreeId)) { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index ab470d78533..60a20f7929f 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3632,6 +3632,9 @@ export class OrcaRuntimeService { }) => AgentHookAuthorityAttestation | null) | null private readonly retireAgentHookCompatibilityAuthorityFn: ((paneKey: string) => void) | null + private readonly reconcileAgentStatusForEndedProcessFn: + | ((paneKeys: Iterable) => void) + | null private readonly canRecoverPersistentLocalPtysFn: () => boolean private readonly buildAgentHookPtyEnv: (() => Record) | null private readonly getDesktopWindowStatusFn: () => RuntimeDesktopWindowStatus @@ -3713,6 +3716,7 @@ export class OrcaRuntimeService { terminalProvenance: 'current_runtime' | 'restored' }) => AgentHookAuthorityAttestation | null retireAgentHookCompatibilityAuthority?: (paneKey: string) => void + reconcileAgentStatusForEndedProcess?: (paneKeys: Iterable) => void canRecoverPersistentLocalPtys?: () => boolean // Why: codex-home paths for the Agent Session History scan must be sourced // here, not via the window-only registerCoreHandlers path — that path never @@ -3755,6 +3759,7 @@ export class OrcaRuntimeService { deps?.attestAgentHookCompatibilityAuthority ?? null this.retireAgentHookCompatibilityAuthorityFn = deps?.retireAgentHookCompatibilityAuthority ?? null + this.reconcileAgentStatusForEndedProcessFn = deps?.reconcileAgentStatusForEndedProcess ?? null this.canRecoverPersistentLocalPtysFn = deps?.canRecoverPersistentLocalPtys ?? (() => true) // Why: configure the shared AiVault scan cache from a serve-mode-reachable // seam so the aiVault.listSessions RPC includes managed-Codex + WSL sessions @@ -13962,6 +13967,27 @@ export class OrcaRuntimeService { } } + /** Every pane key this PTY could be addressed by. Independent of launch authority: an ordinary + * restored PTY has neither a launch token nor a receipt, and those are exactly the panes whose + * spawn-time `ptyPaneKey` mapping teardown could not resolve. */ + private collectPaneKeysForPty(ptyId: string): Set { + const paneKeys = new Set() + const pty = this.ptysById.get(ptyId) + if (pty?.paneKey && parsePaneKey(pty.paneKey)) { + paneKeys.add(pty.paneKey) + } + const receipt = this.restoredOrchestrationAuthorityByPtyId.get(ptyId) + if (receipt?.paneKey && parsePaneKey(receipt.paneKey)) { + paneKeys.add(receipt.paneKey) + } + for (const leaf of this.getLeavesForPty(ptyId)) { + if (isValidTerminalTabId(leaf.tabId) && isTerminalLeafId(leaf.leafId)) { + paneKeys.add(makePaneKey(leaf.tabId, leaf.leafId)) + } + } + return paneKeys + } + private retirePtyAgentLaunchAuthority(ptyId: string): void { const pty = this.ptysById.get(ptyId) if (!pty) { @@ -13971,21 +13997,10 @@ export class OrcaRuntimeService { if (!pty.launchToken && !receipt) { return } + const paneKeys = this.collectPaneKeysForPty(ptyId) this.restoredOrchestrationAuthorityByPtyId.delete(ptyId) pty.launchToken = null pty.launchIncarnationId = null - const paneKeys = new Set() - if (pty.paneKey && parsePaneKey(pty.paneKey)) { - paneKeys.add(pty.paneKey) - } - if (receipt?.paneKey && parsePaneKey(receipt.paneKey)) { - paneKeys.add(receipt.paneKey) - } - for (const leaf of this.getLeavesForPty(ptyId)) { - if (isValidTerminalTabId(leaf.tabId) && isTerminalLeafId(leaf.leafId)) { - paneKeys.add(makePaneKey(leaf.tabId, leaf.leafId)) - } - } for (const paneKey of paneKeys) { this.retireAgentHookCompatibilityAuthorityFn?.(paneKey) } @@ -15201,7 +15216,14 @@ export class OrcaRuntimeService { ptyId: string, exitCode: number, exitIncarnationId?: PtyIncarnationId, - options?: { hostExitConfirmed?: boolean; cause?: TerminalExitCause } + options?: { + hostExitConfirmed?: boolean + cause?: TerminalExitCause + /** The provider's own physical-exit callback fired. Separate from `hostExitConfirmed`, which + * also drives the SSH surface decision and the liveness verdict: node-pty reports real exits + * as -1, so the numeric code alone cannot tell a dead process from a failed stop. */ + providerExitObserved?: boolean + } ): void { const pty = this.ptysById.get(ptyId) if (exitIncarnationId && pty?.incarnationId && exitIncarnationId !== pty.incarnationId) { @@ -15227,6 +15249,9 @@ export class OrcaRuntimeService { pty?.connectionId != null && exitCode < 0 && options?.hostExitConfirmed !== true + // Why: collect before retirePtyAgentLaunchAuthority, which deletes the restored-authority + // receipt a receipt-only pane's key comes from. + const exitPaneKeys = this.collectPaneKeysForPty(ptyId) if (preservesAbnormalSshSurface) { if (this.getPtyLivenessVerdict(ptyId)?.status !== 'unverifiable') { this.markPtyLivenessUnverifiable(ptyId, SSH_EXIT_UNCONFIRMED_REASON) @@ -15235,6 +15260,15 @@ export class OrcaRuntimeService { } else { this.retirePtyAgentLaunchAuthority(ptyId) } + // Why: a synthetic -1 from a failed or unverified stop is not a death certificate (the PTY can + // have survived it), while a real exit can also report -1 — so neither the numeric code nor the + // SSH surface predicate is sufficient on its own. Decided separately from + // preservesAbnormalSshSurface, which a host-confirmed negative SSH exit would otherwise skip. + const processDeathCertified = + exitCode >= 0 || options?.hostExitConfirmed === true || options?.providerExitObserved === true + if (processDeathCertified && exitPaneKeys.size > 0) { + this.reconcileAgentStatusForEndedProcessFn?.(exitPaneKeys) + } const incarnationId = exitIncarnationId ?? pty?.incarnationId ?? diff --git a/src/main/runtime/pty-exit-agent-status-reconciliation.test.ts b/src/main/runtime/pty-exit-agent-status-reconciliation.test.ts new file mode 100644 index 00000000000..f516e4f5b6d --- /dev/null +++ b/src/main/runtime/pty-exit-agent-status-reconciliation.test.ts @@ -0,0 +1,91 @@ +/** + * A PTY that exits without a resolvable spawn-time pane key never reaches + * `agentHookServer.clearPaneState`, so its row and every Claude latch outlive the process with + * nothing left to retire them (STA-4612). `onPtyExit` knows the pane keys that teardown could not + * resolve, so it reconciles them — but only on a real death certificate. + */ +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import { makePaneKey } from '../../shared/stable-pane-id' + +const LEAF = '11111111-1111-4111-8111-111111111111' +const PANE = makePaneKey('tab-1', LEAF) +const PTY = 'wt-1__pty-1' + +type RuntimeInternals = { + ptysById: Map +} + +function runtimeWithBoundPane( + reconcile: (paneKeys: Iterable) => void, + options: { connectionId?: string } = {} +): OrcaRuntimeService { + const runtime = new OrcaRuntimeService(null, undefined, { + reconcileAgentStatusForEndedProcess: reconcile + }) + const ptys = (runtime as unknown as RuntimeInternals).ptysById + ptys.set(PTY, { + ptyId: PTY, + paneKey: PANE, + worktreeId: 'wt-1', + tabId: 'tab-1', + ...(options.connectionId ? { connectionId: options.connectionId } : {}) + } as never) + return runtime +} + +function reconciledPaneKeys(calls: Iterable[]): string[] { + return calls.flatMap((keys) => [...keys]) +} + +describe('onPtyExit agent-status reconciliation', () => { + it('reconciles on a normal zero exit code', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, 0) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('reconciles a physical -1 exit reported by the provider callback', () => { + // node-pty forwards real exits as -1 (local-pty-provider-shutdown.test.ts pins this), so the + // numeric code alone cannot separate a dead process from a failed stop. + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, -1, undefined, { providerExitObserved: true }) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('reconciles a host-confirmed negative exit', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, -1, undefined, { hostExitConfirmed: true }) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('reconciles a provider-observed -1 on a pane bound to a transport, where the SSH surface is preserved', () => { + // A WSL pane carries a `wsl:*` connectionId, so a negative exit keeps the abnormal-SSH surface + // (liveness unverifiable, launch authority not retired) — but the daemon provider's own exit + // callback still witnessed the process die. The two decisions are deliberately independent; + // without this the connectionId arm of the certificate is never exercised. + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile, { connectionId: 'wsl:Ubuntu' }).onPtyExit(PTY, -1, undefined, { + providerExitObserved: true + }) + + expect(reconciledPaneKeys(reconcile.mock.calls.map(([keys]) => keys))).toContain(PANE) + }) + + it('does NOT reconcile a transport-bound synthetic -1, whose remote PTY is designed to survive it', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile, { connectionId: 'ssh-conn-1' }).onPtyExit(PTY, -1) + + expect(reconcile).not.toHaveBeenCalled() + }) + + it('does NOT reconcile a synthetic -1 from a failed stop, whose PTY may have survived', () => { + const reconcile = vi.fn() + runtimeWithBoundPane(reconcile).onPtyExit(PTY, -1) + + expect(reconcile).not.toHaveBeenCalled() + }) +}) diff --git a/src/preload/api/agent-status-api.ts b/src/preload/api/agent-status-api.ts index 900b3ea4490..fd6da88e351 100644 --- a/src/preload/api/agent-status-api.ts +++ b/src/preload/api/agent-status-api.ts @@ -30,6 +30,9 @@ export type AgentStatusApi = { getMigrationUnsupportedSnapshot: () => Promise /** Drop a paneKey from the main-process hook cache and on-disk last-status file. Fire-and-forget. */ drop: (paneKey: string) => void + /** Retire a pane whose agent process is proven gone — clears the row AND the per-pane caches a + * dismissal deliberately keeps. Not `drop`: that one is a user dismissal of a live pane's row. */ + reconcileEndedProcess: (paneKey: string) => void /** Drop every cached hook status under one terminal tab prefix. Fire-and-forget. */ dropByTabPrefix: (tabId: string) => void /** Permanently retire one pane's hook authority while siblings stay live. */ diff --git a/src/preload/index.ts b/src/preload/index.ts index b8a00c6d647..4c30124a6da 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -5032,6 +5032,9 @@ const api = { drop: (paneKey: string): void => { ipcRenderer.send('agentStatus:drop', paneKey) }, + reconcileEndedProcess: (paneKey: string): void => { + ipcRenderer.send('agentStatus:reconcileEndedProcess', paneKey) + }, /** Drop all cached hook statuses under one terminal tab prefix; fired on explicit tab close even without a local row. */ dropByTabPrefix: (tabId: string): void => { ipcRenderer.send('agentStatus:dropByTabPrefix', tabId) diff --git a/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx b/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx index 990d9219627..9acf590819f 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.recent-tabs.test.tsx @@ -12,6 +12,7 @@ import { makePaneKey } from '../../../shared/stable-pane-id' import { LEAF_ID, makeAgentEntry, + makeDuplicateRecentTabState, makeGroup, makeManyTabState, makeRecentTabState, @@ -229,6 +230,25 @@ describe('WorktreeJumpPalette recent chats & terminals', () => { expect(testContainer.textContent).toContain('Recent Worktrees') }) + it('keeps duplicate persisted tab ids as separate recent rows and digit targets', async () => { + await renderPalette(makeDuplicateRecentTabState()) + + expect( + getRenderedRowIds().filter( + (id) => id === 'workspace-tab:tab-duplicate' || id.includes(':workspace-tab:tab-duplicate') + ) + ).toEqual(['workspace-tab:tab-duplicate', 'palette-dup:1:workspace-tab:tab-duplicate']) + + await act(async () => { + emitCmdJRowIndexJump(1) + }) + await flushEffects() + + expect(activateWorkspaceTabPaletteResult).toHaveBeenCalledWith( + expect.objectContaining({ tabId: 'tab-duplicate', worktreeId: 'wt-beta' }) + ) + }) + it('caps the recent section so the worktree header stays above the fold', async () => { await renderPalette(makeManyTabState(12)) diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index 9a54ed398c7..0d69d1a9a68 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -350,9 +350,56 @@ function isCurrentOpenTabItem(item: OpenTabPaletteItem): boolean { return item.type === 'browser-page' ? item.result.isCurrentPage : item.result.isCurrentTab } +/** Not the command id: two hosts — or a duplicate snapshot — can publish the same tab id. */ +function getRecentTabOccurrenceBase(item: OpenTabPaletteItem): string { + if (item.type === 'browser-page') { + const result = item.result + return JSON.stringify([ + item.type, + item.id, + result.executionHostId ?? '', + result.worktreeId, + result.workspaceId, + result.pageId + ]) + } + if (item.type === 'simulator-tab') { + const result = item.result + // Why no groupId: it changes when a tab is regrouped mid-open, and the frozen + // order must keep resolving the row; tabId already identifies it within a host. + return JSON.stringify([ + item.type, + item.id, + result.executionHostId ?? '', + result.worktreeId, + result.tabId + ]) + } + const result = item.result + return JSON.stringify([ + item.type, + item.id, + result.executionHostId ?? '', + result.worktreeId, + result.tabId, + result.entityId + ]) +} + +function buildRecentTabOccurrenceIds(items: readonly OpenTabPaletteItem[]): string[] { + const nextOrdinalByBase = new Map() + return items.map((item) => { + const base = getRecentTabOccurrenceBase(item) + const ordinal = nextOrdinalByBase.get(base) ?? 0 + nextOrdinalByBase.set(base, ordinal + 1) + return `recent-tab:${base}:${ordinal}` + }) +} + /** An open tab's recent-section row plus the inputs inclusion needs. */ type OpenTabRecentRow = { item: OpenTabPaletteItem + occurrenceId: string worktree: Worktree row: RecentWorkspaceTabRow } @@ -1419,17 +1466,22 @@ function WorktreeJumpPaletteContent({ // once inclusion would drop it (a current tab that quiets down), or the pip blanks mid-open. const openTabRecentRows = useMemo(() => { const entries: OpenTabRecentRow[] = [] - for (const item of openTabItems) { + const occurrenceIds = buildRecentTabOccurrenceIds(openTabItems) + for (const [index, item] of openTabItems.entries()) { const worktree = resolveWorktree(item.result.worktreeId, item.result.executionHostId) if (!worktree) { continue } + const occurrenceId = occurrenceIds[index] entries.push({ item, + occurrenceId, worktree, row: { id: item.id, + occurrenceId, worktreeId: worktree.id, + worktreeHostId: worktree.hostId, unifiedTabId: item.type === 'browser-page' ? null : item.result.tabId, terminalTab: item.type === 'workspace-tab' && item.result.contentType === 'terminal' @@ -1442,8 +1494,8 @@ function WorktreeJumpPaletteContent({ return entries }, [openTabItems, resolveWorktree, terminalTabsById]) - const recentTabRowById = useMemo( - () => new Map(openTabRecentRows.map(({ row }) => [row.id, row])), + const recentTabRowByItem = useMemo( + () => new Map(openTabRecentRows.map(({ item, row }) => [item, row])), [openTabRecentRows] ) @@ -1558,15 +1610,16 @@ function WorktreeJumpPaletteContent({ visible ]) - // Why: walk the frozen order rather than re-sorting the tab list — the frozen ids are the ranking, - // so agent churn never reshuffles rows under the cursor. Cap after resolving, not before: a chip - // applied mid-open narrows `openTabItems`, and capping first would leave the section empty. + // Why walk the frozen order, and cap after resolving: agent churn must not reshuffle rows, and a + // mid-open chip narrows `openTabItems` — capping first would leave the section empty. const recentTabItems = useMemo(() => { - const itemById = new Map(openTabItems.map((item) => [item.id, item])) + const itemByOccurrenceId = new Map( + openTabRecentRows.map(({ occurrenceId, item }) => [occurrenceId, item]) + ) return recentTabOrder - .flatMap((id) => itemById.get(id) ?? []) + .flatMap((occurrenceId) => itemByOccurrenceId.get(occurrenceId) ?? []) .slice(0, EMPTY_QUERY_RECENT_TAB_CAP) - }, [openTabItems, recentTabOrder]) + }, [openTabRecentRows, recentTabOrder]) const settingsResults = useMemo( () => buildCmdJSettingsResults(settingsSections), @@ -1873,10 +1926,10 @@ function WorktreeJumpPaletteContent({ ]) // Why: badges number the snapshotted recent rows only — ⌘N is meaningless on a typed query. - const recentTabShortcutIndexById = useMemo( + const recentTabShortcutIndexByItem = useMemo( () => new Map( - hasQuery ? [] : paletteSections.visibleOpenTabItems.map((item, index) => [item.id, index]) + hasQuery ? [] : paletteSections.visibleOpenTabItems.map((item, index) => [item, index]) ), [hasQuery, paletteSections] ) @@ -3450,7 +3503,7 @@ function WorktreeJumpPaletteContent({ ) // Why regardless of query: a searched-for tab is exactly when you need to know it's // still working — the map covers every open tab, not just the recent section. - const recentRow = recentTabRowById.get(entry.id) ?? null + const recentRow = recentTabRowByItem.get(entry) ?? null return ( )} @@ -3594,7 +3647,7 @@ function WorktreeJumpPaletteContent({ )} @@ -3672,7 +3725,7 @@ function WorktreeJumpPaletteContent({ )} diff --git a/src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts b/src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts index 1993e044f4b..214b6c2c72c 100644 --- a/src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts +++ b/src/renderer/src/components/sidebar/active-worktree-focus-after-delete.ts @@ -4,6 +4,7 @@ import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { isRuntimeOwnedSshTargetId, parseExecutionHostId } from '../../../../shared/execution-host' import type { Repo } from '../../../../shared/repo-types' import type { Worktree } from '../../../../shared/worktree/types' +import { getWorktreeVisitTimestamp } from '@/lib/worktree-visit-recency' import { getDeleteStateForWorktreeHost } from './worktree-delete-state-host-match' type AppStoreState = ReturnType @@ -56,7 +57,9 @@ function pickNextWorktreeIdAfterDelete( if (others.length > 0) { const lastVisited = state.lastVisitedAtByWorktreeId const [mostRecent] = [...others].sort( - (a, b) => (lastVisited[b.id] ?? 0) - (lastVisited[a.id] ?? 0) + (a, b) => + (getWorktreeVisitTimestamp(lastVisited, b) ?? 0) - + (getWorktreeVisitTimestamp(lastVisited, a) ?? 0) ) return mostRecent.id } diff --git a/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts index dc3a6c4e2d5..5a6de84e03c 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-command-finished-cleanup.test.ts @@ -328,6 +328,106 @@ describe('connectPanePty', () => { shellForeground: true }) expect(resolveMockPaneWindowsShiftEnterEncoding(mockStoreState, paneKey)).toBe('alt-enter') + // Why: `agentStatus:drop` deliberately preserves a live pane's per-pane caches, so a + // process-confirmed agent exit must go through the reconcile route instead — otherwise a + // surviving Claude latch resolves the pane's next event straight back to 'working' (STA-4612). + expect(window.api.agentStatus.reconcileEndedProcess).toHaveBeenCalledWith(paneKey) + }) + + it('reconciles a pane that HAS a status row, whose own drop removes it first', async () => { + // Why this shape: the sibling test above covers a pane with no row, where the deferred drop + // early-returns and never touches the pane's accepted-status bookkeeping. That is not the + // production case. A pane worth reconciling always HAS a row — and settling runs the drop + // BEFORE the reconcile, so the reconcile must survive its own drop having already fired. + vi.useFakeTimers() + const { connectPanePty } = await import('./pty-connection') + // Both imported after vi.resetModules() so they share pty-connection's module registry; a + // static top-level import here is a DIFFERENT instance and the assertion goes vacuous. + const { createTestStore } = await import('@/store/slices/store-test-helpers') + vi.mocked(window.api.pty.confirmForegroundProcess).mockResolvedValue('powershell.exe') + vi.mocked(window.api.agentStatus.reconcileEndedProcess).mockClear() + const dataCallbackRef: { current: ((data: string) => void) | null } = { current: null } + const ptyId = 'pty-claude-row-confirmed-shell' + const transport = createMockTransport(ptyId) + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + dataCallbackRef.current = callbacks.onData ?? null + return { id: ptyId } + }) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ isVisibleRef: { current: false } }) as never + ) + await vi.advanceTimersByTimeAsync(20) + await flushAsyncTicks() + + // A real accepted status write, through the real slice, so the row carries whatever + // bookkeeping the store stamps on an accepted write. `working` is the shape STA-4612 is about: + // a latch is holding the pane open and the agent is about to exit out from under it. + const realStore = createTestStore() + realStore + .getState() + .setAgentStatus(paneKey, { state: 'working', prompt: 'ship it', agentType: 'claude' }) + const row = realStore.getState().agentStatusByPaneKey[paneKey] + expect(row).toBeDefined() + mockStoreState.agentStatusByPaneKey[paneKey] = row + // The real dismissal removes the row and runs the slice's own teardown; mirror both, or the + // ordering this test exists to pin is never exercised. + mockStoreState.dropAgentStatus.mockImplementation((key: string) => { + delete mockStoreState.agentStatusByPaneKey[key] + realStore.getState().dropAgentStatus(key) + }) + + dataCallbackRef.current?.('\x1b]133;D;0\x07') + await vi.advanceTimersByTimeAsync(350) + + expect(mockStoreState.dropAgentStatus).toHaveBeenCalledWith(paneKey) + expect(window.api.agentStatus.reconcileEndedProcess).toHaveBeenCalledWith(paneKey) + }) + + it('does NOT reconcile when the process check could not confirm a shell', async () => { + vi.useFakeTimers() + const { connectPanePty } = await import('./pty-connection') + vi.mocked(window.api.pty.confirmForegroundProcess).mockResolvedValue(null) + vi.mocked(window.api.agentStatus.reconcileEndedProcess).mockClear() + const dataCallbackRef: { current: ((data: string) => void) | null } = { current: null } + const ptyId = 'pty-unconfirmed-shell' + const transport = createMockTransport(ptyId) + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + dataCallbackRef.current = callbacks.onData ?? null + return { id: ptyId } + }) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + + connectPanePty( + createPane(1) as never, + createManager(1) as never, + createDeps({ isVisibleRef: { current: false } }) as never + ) + await vi.advanceTimersByTimeAsync(20) + await flushAsyncTicks() + mockStoreState.agentLaunchConfigByPaneKey[paneKey] = { + launchConfig: { agentArgs: '', agentEnv: {} }, + identity: { agentType: 'droid' } + } + + dataCallbackRef.current?.('\x1b]133;D;0\x07') + // Run the whole confirm ladder. At 350ms the unavailable branch has not settled yet, so + // asserting there proves nothing — it would pass just as happily if this path DID tear the + // pane down a moment later. + await vi.advanceTimersByTimeAsync(350 + 1200 + 6000) + await flushAsyncTicks() + + // The unavailable branch really ran: it publishes an unproven foreground before settling. + expect(mockStoreState.paneForegroundAgentByPaneKey[paneKey]).toEqual({ + agent: null, + shellForeground: false + }) + expect(window.api.agentStatus.reconcileEndedProcess).not.toHaveBeenCalled() }) it('disarms stale TUI modes in the emulator after a confirmed return to shell', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts b/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts index 6e9f24f3efe..eb3a40b6803 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-test-environment.ts @@ -72,7 +72,8 @@ export function installTerminalTestGlobals(): void { restoreTerminalFit: vi.fn().mockResolvedValue({ restored: true }) }, agentStatus: { - inferInterrupt: vi.fn().mockResolvedValue(false) + inferInterrupt: vi.fn().mockResolvedValue(false), + reconcileEndedProcess: vi.fn() } }, dispatchEvent: vi.fn(), diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts index 9f1b2b7ebe7..da59cd06c79 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts @@ -88,12 +88,45 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { } } session.deferredCommandFinishedStatusDrop = null + session.deferredConfirmedShellReconcile = null + /** The pane's foreground was proven to be a shell, so its agent exited. Unlike a user dismissal, + * this must also retire the main-side per-pane caches — a surviving Claude latch resolves the + * next event straight back to `working`. + * + * Ordered by the row's `acceptedStatusSeq`, not by row identity or `updatedAt`: the confirming + * process read can take seconds, an unrelated field moving in that window is not evidence the + * agent is alive, and a genuinely newer row can share the anchor's millisecond. Reading the token + * off the row is what makes the ordering safe — the paired drop runs FIRST and removes the row, + * and a removed row means nothing reported, not "the anchor is gone". + * + * Residual: a row deleted by some OTHER path mid-window and then rebuilt restarts its count, so a + * pane armed at 1 that reports exactly once more compares equal. It needs a foreign drop inside + * the confirm window; the relaunch case that would otherwise hit it is already discarded by + * onCommandStarted, and the cost is retiring a pane the process table just proved is a shell. */ + const reconcileEndedProcessIfPaneQuiet = (armedAcceptedStatusSeq: number | undefined): void => { + const current = useAppStore.getState().agentStatusByPaneKey[session.cacheKey] + if (current && current.acceptedStatusSeq !== armedAcceptedStatusSeq) { + return + } + // Why: main-side only. The renderer row and launch config are already owned by the deferred + // drop above; what that path cannot reach is the hook server's per-pane Claude latches, which + // `agentStatus:drop` deliberately preserves for a still-live pane. Main echoes its own clear + // back through the pane-status-cleared channel, so both sides stay consistent. + window.api?.agentStatus?.reconcileEndedProcess?.(session.cacheKey) + } session.visibleForegroundSamplePending = false session.visibleForegroundSampleSettled = false - session.settleDeferredCommandFinishedStatusDrop = (): void => { + session.settleDeferredCommandFinishedStatusDrop = ( + options: { confirmedShell?: boolean } = {} + ): void => { const dropStatus = session.deferredCommandFinishedStatusDrop + const reconcile = session.deferredConfirmedShellReconcile session.deferredCommandFinishedStatusDrop = null + session.deferredConfirmedShellReconcile = null dropStatus?.() + if (options.confirmedShell) { + reconcile?.() + } } session.isForegroundTrackingAllowed = (id: string): boolean => { if (isRemoteRuntimePtyId(id) || parseAppSshPtyId(id) !== null) { @@ -143,9 +176,11 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { useAppStore.getState().clearAgentLaunchConfig(session.cacheKey) return } - session.settleDeferredCommandFinishedStatusDrop() + session.settleDeferredCommandFinishedStatusDrop({ confirmedShell: true }) }, - onCommandFinishedUnavailable: session.settleDeferredCommandFinishedStatusDrop, + // Why wrapped: passed bare, a caller-supplied argument would be read as `options` and could + // reconcile on the unavailable path, which has no proof the agent exited. + onCommandFinishedUnavailable: () => session.settleDeferredCommandFinishedStatusDrop(), onVisibleForegroundSettled: (outcome) => { session.visibleForegroundSamplePending = false session.visibleForegroundSampleSettled = outcome !== 'inconclusive' @@ -195,10 +230,16 @@ export function installPaneAgentIdentity(session: ConnectPanePtySession): void { if (shouldDeferStatusDrop) { // Why: keep the concrete pane identity routable while the local process // check distinguishes a leaked nested-shell D from a genuine agent exit. + // Anchor the accepted-status ordinal once, here — not per rung of the confirm ladder, which + // can span seconds — so the gate tolerates churn across the whole window. + const armedAcceptedStatusSeq = entry?.acceptedStatusSeq session.deferredCommandFinishedStatusDrop = dropStatus + session.deferredConfirmedShellReconcile = () => + reconcileEndedProcessIfPaneQuiet(armedAcceptedStatusSeq) return } session.deferredCommandFinishedStatusDrop = null + session.deferredConfirmedShellReconcile = null dropStatus() } session.sampleVisiblePaneForegroundAgent = (forceRoutingConfirmation = false): void => { diff --git a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.test.tsx b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.test.tsx index 8e946009725..061fdf3ea9e 100644 --- a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.test.tsx +++ b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.test.tsx @@ -9,6 +9,7 @@ import { resetWorkspaceCleanupBrowsePersistTimer } from '@/store/slices/workspace-cleanup-browse' import { createDefaultWorkspaceCleanupBrowseState } from '../../../../shared/workspace-cleanup-browse-state' +import { listAppliedWorkspaceCleanupFilters } from '../../../../shared/workspace-cleanup-applied-filters' import { useWorkspaceCleanupBrowseState } from './use-workspace-cleanup-browse-state' import type { WorkspaceCleanupBrowseController } from './use-workspace-cleanup-browse-state' @@ -104,5 +105,27 @@ describe('useWorkspaceCleanupBrowseState', () => { field: 'size', direction: 'desc' }) +}) + + it('keeps a cleared chip cleared when a facet patch lands in the same tick', () => { + const controller = mountController() + act(() => controller.current!.patchFilters('activity', { idleMinDays: 20 })) + + const format = new Proxy({}, { get: () => () => 'chip' }) as Parameters< + typeof listAppliedWorkspaceCleanupFilters + >[1] + const chip = listAppliedWorkspaceCleanupFilters( + store.getState().workspaceCleanupBrowse.filters, + format + ).find((a) => a.id === 'activity.idleMinDays')! + + act(() => { + controller.current!.replaceFilters(chip.clear) + controller.current!.patchFilters('git', { states: ['dirty'] }) + }) + + const filters = store.getState().workspaceCleanupBrowse.filters + expect(filters.activity.idleMinDays).toBeNull() + expect(filters.git.states).toEqual(['dirty']) }) }) diff --git a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.ts b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.ts index 76f4164b694..3651f4e01a0 100644 --- a/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.ts +++ b/src/renderer/src/components/workspace-cleanup/use-workspace-cleanup-browse-state.ts @@ -16,6 +16,10 @@ export type WorkspaceCleanupBrowseController = { ) => void toggleSortField: (field: WorkspaceCleanupSortField) => void clearFilters: () => void + /** Applies a whole-state transform, for callers that clear one named constraint. */ + replaceFilters: ( + transform: (filters: WorkspaceCleanupFilterState) => WorkspaceCleanupFilterState + ) => void } /** @@ -56,6 +60,15 @@ export function useWorkspaceCleanupBrowseState(): WorkspaceCleanupBrowseControll [updateBrowse] ) + // Same updater form as patchFilters: a chip clear and a facet patch can land in one + // tick, and whichever read the render snapshot would undo the other. + const replaceFilters = useCallback( + (transform) => { + updateBrowse((current) => ({ ...current, filters: transform(current.filters) })) + }, + [updateBrowse] + ) + const clearFilters = useCallback(() => { updateBrowse((current) => ({ ...current, @@ -68,6 +81,7 @@ export function useWorkspaceCleanupBrowseState(): WorkspaceCleanupBrowseControll sort: browse.sort, patchFilters, toggleSortField, - clearFilters + clearFilters, + replaceFilters } } diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-applied-filter-chips.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-applied-filter-chips.tsx new file mode 100644 index 00000000000..38d615514cc --- /dev/null +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-applied-filter-chips.tsx @@ -0,0 +1,180 @@ +import React from 'react' +import { X } from 'lucide-react' +import { Button } from '@/components/ui/button' +import { translate } from '@/i18n/i18n' +import { toMegabytes } from './workspace-cleanup-facet-panel-model' +import type { WorkspaceCleanupAppliedFilter } from '../../../../shared/workspace-cleanup-applied-filters' +import { listAppliedWorkspaceCleanupFilters } from '../../../../shared/workspace-cleanup-applied-filters' +import type { WorkspaceCleanupFilterState } from '../../../../shared/workspace-cleanup-filter-model' + +/** Why in the bar, not the panel: a filter narrowing the list must be visible without opening anything. */ +// Why not memoized: the labels are translated at build time, so caching them on +// `filters` alone reuses the previous language's strings after a language change. +// Derivation is constant-size (one pass over the filter fields, not per row). +export function useAppliedWorkspaceCleanupFilters( + filters: WorkspaceCleanupFilterState +): WorkspaceCleanupAppliedFilter[] { + return listAppliedWorkspaceCleanupFilters(filters, buildAppliedFilterFormatters()) +} + +export function WorkspaceCleanupAppliedFilterChips({ + applied, + onClear +}: { + applied: readonly WorkspaceCleanupAppliedFilter[] + onClear: (filter: WorkspaceCleanupAppliedFilter) => void +}): React.JSX.Element | null { + if (applied.length === 0) { + return null + } + return ( +
+ {applied.map((filter) => ( + + {filter.label} + + + ))} +
+ ) +} + +function buildAppliedFilterFormatters(): Parameters[1] { + const mb = (bytes: number): number => toMegabytes(bytes) ?? 0 + return { + idleDays: (days) => + translate('components.workspace.cleanup.browse.chip.idleDays', 'Idle {{value0}}d+', { + value0: days + }), + neverVisited: () => + translate('components.workspace.cleanup.browse.chip.neverVisited', 'Never visited'), + minSize: (bytes) => + translate('components.workspace.cleanup.browse.chip.minSize', 'At least {{value0}} MB', { + value0: mb(bytes) + }), + maxSize: (bytes) => + translate('components.workspace.cleanup.browse.chip.maxSize', 'At most {{value0}} MB', { + value0: mb(bytes) + }), + excludesUnsized: () => + translate('components.workspace.cleanup.browse.chip.excludesUnsized', 'Measured only'), + excludesStatusless: () => + translate('components.workspace.cleanup.browse.chip.excludesStatusless', 'Has a status'), + list: (kind, count) => + translate('components.workspace.cleanup.browse.chip.list', '{{value0}}: {{value1}}', { + value0: getChipKindLabel(kind), + value1: count + }), + triState: (kind, mode) => + translate('components.workspace.cleanup.browse.chip.triState', '{{value0}}: {{value1}}', { + value0: getChipKindLabel(kind), + value1: + mode === 'only' + ? translate('components.workspace.cleanup.browse.chip.only', 'Only') + : translate('components.workspace.cleanup.browse.chip.exclude', 'Excluded') + }), + minAhead: (count) => + translate('components.workspace.cleanup.browse.chip.minAhead', 'Ahead {{value0}}+', { + value0: count + }), + minBehind: (count) => + translate('components.workspace.cleanup.browse.chip.minBehind', 'Behind {{value0}}+', { + value0: count + }), + branchQuery: (value) => + translate('components.workspace.cleanup.browse.chip.branchQuery', 'Branch: {{value0}}', { + value0: value + }), + pathPrefix: (value) => + translate('components.workspace.cleanup.browse.chip.pathPrefix', 'Path: {{value0}}', { + value0: value + }), + presence: (kind, mode) => + translate('components.workspace.cleanup.browse.chip.presence', '{{value0}}: {{value1}}', { + value0: getChipKindLabel(kind), + value1: + mode === 'some' + ? translate('components.workspace.cleanup.browse.chip.has', 'Has') + : translate('components.workspace.cleanup.browse.chip.none', 'None') + }), + completelyEmpty: () => + translate('components.workspace.cleanup.browse.chip.completelyEmpty', 'Nothing to lose'), + selectableOnly: () => + translate('components.workspace.cleanup.browse.chip.selectableOnly', 'Deletable only') + } +} + +function getChipKindLabel(kind: string): string { + switch (kind) { + case 'status': + return translate('components.workspace.cleanup.browse.chip.kind.status', 'Status') + case 'agent': + return translate('components.workspace.cleanup.browse.chip.kind.agent', 'Agent') + case 'git': + return translate('components.workspace.cleanup.browse.chip.kind.git', 'Git') + case 'review': + return translate('components.workspace.cleanup.browse.chip.kind.review', 'Review') + case 'reviewState': + return translate('components.workspace.cleanup.browse.chip.kind.reviewState', 'Review state') + case 'reviewProvider': + return translate('components.workspace.cleanup.browse.chip.kind.reviewProvider', 'Provider') + case 'ticket': + return translate('components.workspace.cleanup.browse.chip.kind.ticket', 'Ticket') + case 'ticketSource': + return translate('components.workspace.cleanup.browse.chip.kind.ticketSource', 'Ticket source') + case 'context': + return translate('components.workspace.cleanup.browse.chip.kind.context', 'Context') + case 'host': + return translate('components.workspace.cleanup.browse.chip.kind.host', 'Host') + case 'repo': + return translate('components.workspace.cleanup.browse.chip.kind.repo', 'Repo') + case 'blocker': + return translate('components.workspace.cleanup.browse.chip.kind.blocker', 'Blocker') + case 'tier': + return translate('components.workspace.cleanup.browse.chip.kind.tier', 'Safety') + case 'dismissed': + return translate('components.workspace.cleanup.browse.chip.kind.dismissed', 'Ignored') + case 'archived': + return translate('components.workspace.cleanup.browse.chip.kind.archived', 'Archived') + case 'pinned': + return translate('components.workspace.cleanup.browse.chip.kind.pinned', 'Pinned') + case 'unread': + return translate('components.workspace.cleanup.browse.chip.kind.unread', 'Unread') + case 'comment': + return translate('components.workspace.cleanup.browse.chip.kind.comment', 'Comment') + case 'prunable': + return translate('components.workspace.cleanup.browse.chip.kind.prunable', 'Prunable') + case 'locked': + return translate('components.workspace.cleanup.browse.chip.kind.locked', 'Locked') + case 'retainedAgents': + return translate( + 'components.workspace.cleanup.browse.chip.kind.retainedAgents', + 'Finished agents' + ) + default: + return kind + } +} diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-browse-toolbar.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-browse-toolbar.tsx index c70209dd0eb..d53b3215c2c 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-browse-toolbar.tsx +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-browse-toolbar.tsx @@ -71,6 +71,7 @@ export function WorkspaceCleanupBrowseToolbar({ gitEvidence={{ pendingCount: gitPendingCount, totalCount: gitCheckedTotal }} onQueryChange={(query) => browse.patchFilters('query', query)} onClearFilters={browse.clearFilters} + onClearAppliedFilter={(filter) => browse.replaceFilters(filter.clear)} /> + hasActiveFilters?: boolean + onClearAppliedFilter?: (filter: WorkspaceCleanupAppliedFilter) => void + onClearFilters?: () => void + } = {} +): void { render( ) } @@ -46,6 +56,41 @@ describe('WorkspaceCleanupFilterBar', () => { expect(screen.queryByRole('button', { name: 'Scan' })).toBeNull() }) + it('names an applied filter in the bar without opening the panel', () => { + // The reported defect: a persisted idleMinDays the user never set. The bar already + // read "Showing N of M", so only the cause was hidden. + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 20 + renderFilterBar(false, { filters, hasActiveFilters: true }) + + expect(screen.getByRole('listitem').textContent).toContain('Idle 20d+') + }) + + it('clears one named filter from its chip', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 20 + const onClearAppliedFilter = vi.fn() + renderFilterBar(false, { filters, hasActiveFilters: true, onClearAppliedFilter }) + + screen.getByRole('button', { name: 'Remove filter Idle 20d+' }).click() + + expect(onClearAppliedFilter).toHaveBeenCalledTimes(1) + expect(onClearAppliedFilter.mock.calls[0][0].id).toBe('activity.idleMinDays') + }) + + it('offers Clear filters in the bar, not only inside the panel', () => { + renderFilterBar(false, { hasActiveFilters: true }) + + expect(screen.getAllByRole('button', { name: /Clear filters/ }).length).toBeGreaterThan(0) + }) + + it('shows no chips and no Clear for a default profile', () => { + renderFilterBar() + + expect(screen.queryAllByRole('listitem')).toHaveLength(0) + expect(screen.queryByRole('button', { name: /Clear filters/ })).toBeNull() + }) + it('keeps the footer visible while the facet panel scrolls', () => { renderFilterBar(true) diff --git a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx index 1fcfa2049dc..3f4ae9e9ab9 100644 --- a/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx +++ b/src/renderer/src/components/workspace-cleanup/workspace-cleanup-filter-bar.tsx @@ -5,6 +5,11 @@ import { Input } from '@/components/ui/input' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { ScrollArea } from '@/components/ui/scroll-area' import { translate } from '@/i18n/i18n' +import { + useAppliedWorkspaceCleanupFilters, + WorkspaceCleanupAppliedFilterChips +} from './workspace-cleanup-applied-filter-chips' +import type { WorkspaceCleanupAppliedFilter } from '../../../../shared/workspace-cleanup-applied-filters' import { WorkspaceCleanupGitReviewFacets } from './workspace-cleanup-git-review-facets' import { WorkspaceCleanupLifecycleFacets } from './workspace-cleanup-lifecycle-facets' import type { WorkspaceCleanupFacetGroupProps } from './workspace-cleanup-facet-panel-model' @@ -23,7 +28,8 @@ export function WorkspaceCleanupFilterBar({ hasActiveFilters, gitEvidence, onQueryChange, - onClearFilters + onClearFilters, + onClearAppliedFilter }: { facetProps: WorkspaceCleanupFacetGroupProps facetPanelOpen: boolean @@ -34,8 +40,10 @@ export function WorkspaceCleanupFilterBar({ gitEvidence: WorkspaceCleanupGitEvidenceProgress onQueryChange: (query: string) => void onClearFilters: () => void + onClearAppliedFilter: (filter: WorkspaceCleanupAppliedFilter) => void }): React.JSX.Element { const { filters, totalCount } = facetProps + const applied = useAppliedWorkspaceCleanupFilters(filters) return (
@@ -101,6 +109,15 @@ export function WorkspaceCleanupFilterBar({ )} + {/* Why out here and not only in the popover: a filter narrowing the list must be + clearable without first discovering which panel it lives in. */} + {hasActiveFilters ? ( + + ) : null} + {gitEvidence.pendingCount > 0 ? ( @@ -111,6 +128,8 @@ export function WorkspaceCleanupFilterBar({ )} ) : null} + +
) } diff --git a/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts b/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts index 5dfbcdaf6a8..19c97ada254 100644 --- a/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts +++ b/src/renderer/src/components/worktree-jump-palette-test-fixtures.ts @@ -136,6 +136,36 @@ export function makeRecentTabState(overrides: Partial = {}): Partial { + const alpha = makeWorktree('wt-alpha', 'Alpha workspace', { hostId: 'ssh:alpha' }) + const beta = makeWorktree('wt-beta', 'Beta workspace', { hostId: 'ssh:beta' }) + return { + worktreesByRepo: { 'repo-1': [alpha, beta] }, + showSleepingWorkspaces: true, + ptyIdsByTabId: { + 'term-alpha': ['pty-term-alpha'], + 'term-beta': ['pty-term-beta'] + }, + tabsByWorktree: { + 'wt-alpha': [makeTerminalTab('term-alpha', 'wt-alpha', 'Alpha duplicate')], + 'wt-beta': [makeTerminalTab('term-beta', 'wt-beta', 'Beta duplicate')] + }, + unifiedTabsByWorktree: { + 'wt-alpha': [makeUnifiedTab('tab-duplicate', 'wt-alpha', 'term-alpha', 'Alpha duplicate')], + 'wt-beta': [makeUnifiedTab('tab-duplicate', 'wt-beta', 'term-beta', 'Beta duplicate')] + }, + groupsByWorktree: { + 'wt-alpha': [makeGroup('wt-alpha', ['tab-duplicate'])], + 'wt-beta': [makeGroup('wt-beta', ['tab-duplicate'])] + }, + activeGroupIdByWorktree: { + 'wt-alpha': 'group-wt-alpha', + 'wt-beta': 'group-wt-beta' + } + } +} + /** One tab-heavy worktree plus `count` bare ones, so both sections overflow their caps. */ export function makeManyTabState(count: number): Partial { const ids = Array.from({ length: count }, (_, index) => `${index}`) diff --git a/src/renderer/src/hooks/remote-workspace-session-merge.ts b/src/renderer/src/hooks/remote-workspace-session-merge.ts index 4a2b087817f..731b1412068 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge.ts @@ -1,7 +1,12 @@ import type { TerminalTab } from '../../../shared/terminal-tab-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { ExecutionHostId } from '../../../shared/execution-host' import { worktreeWorkspaceKey } from '../../../shared/workspace-scope' import { splitWorktreeId } from '../../../shared/worktree/id' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' import type { AppState } from '../store/types' function preserveNewerLocalTerminalFields(remote: TerminalTab, local: TerminalTab): TerminalTab { @@ -20,7 +25,8 @@ export function mergeDirectSshRemoteWorkspaceSession( remote: WorkspaceSessionState, replaceWorktreeIds: ReadonlySet, liveTabsByWorktree: AppState['tabsByWorktree'], - preserveLocalTerminalTabIds: ReadonlySet + preserveLocalTerminalTabIds: ReadonlySet, + replaceExecutionHostId?: ExecutionHostId ): WorkspaceSessionState { const currentTabsById = new Map( [...replaceWorktreeIds] @@ -146,6 +152,25 @@ export function mergeDirectSshRemoteWorkspaceSession( Object.fromEntries( Object.entries(record ?? {}).filter(([worktreeId]) => !replaceWorktreeIds.has(worktreeId)) ) + const omitTargetVisitRecency = ( + record: Record | undefined + ): Record => + Object.fromEntries( + Object.entries(record ?? {}).filter(([key]) => { + const worktreeId = isWorktreeHostIdentity(key) ? getWorktreeIdFromHostIdentity(key) : key + if (!replaceWorktreeIds.has(worktreeId)) { + return true + } + // A direct SSH snapshot replaces one host's row. Legacy bare keys + // remain because they may be the only recency evidence for a sibling + // host; a qualified key is removed only for the target host. + return Boolean( + replaceExecutionHostId && + (!isWorktreeHostIdentity(key) || + key.slice(0, key.indexOf('|')) !== replaceExecutionHostId) + ) + }) + ) const terminalLayoutsByTabId = { ...Object.fromEntries( Object.entries(current.terminalLayoutsByTabId).filter( @@ -226,7 +251,7 @@ export function mergeDirectSshRemoteWorkspaceSession( ) }, lastVisitedAtByWorktreeId: { - ...omitTargetWorktrees(current.lastVisitedAtByWorktreeId), + ...omitTargetVisitRecency(current.lastVisitedAtByWorktreeId), ...remote.lastVisitedAtByWorktreeId }, defaultTerminalTabsAppliedByWorktreeId: { diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts b/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts index 1ffad891d89..16929fd6379 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts @@ -2,6 +2,7 @@ import type { StoreApi } from 'zustand' import type { RemoteWorkspaceSnapshot } from '../../../shared/remote-workspace-types' import { importRemoteWorkspaceSession } from '../../../shared/remote-workspace-session-projection' import type { DirectSshAuthority } from '../../../shared/ssh-types' +import { toSshExecutionHostId } from '../../../shared/execution-host' import { translate } from '@/i18n/i18n' import { buildWorkspaceSessionPayload } from '../lib/workspace-session' import { resolveDirectSshTargetScope } from '../lib/direct-ssh-target-scope' @@ -109,14 +110,16 @@ export async function applyDirectSshRemoteWorkspaceSnapshot({ const state = store.getState() const worktreeIds = exactTargetWorktreeIds(state, authority) const remoteSession = importRemoteWorkspaceSession(snapshot.session, { - resolveWorktreeId: uniqueWorktreeIdByPath(worktreeIds) + resolveWorktreeId: uniqueWorktreeIdByPath(worktreeIds), + executionHostId: toSshExecutionHostId(authority.targetId) }) const merged = mergeDirectSshRemoteWorkspaceSession( buildWorkspaceSessionPayload(state), remoteSession, worktreeIds, state.tabsByWorktree, - currentRecoveryTabIds(state, authority, worktreeIds) + currentRecoveryTabIds(state, authority, worktreeIds), + toSshExecutionHostId(authority.targetId) ) if (!isArrivalCurrent(authority.targetId, arrival) || !isPreparationTokenCurrent(token)) { return diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index f4700df44b3..b04c3cd92de 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -924,7 +924,12 @@ export function useIpcEvents(): void { `[worktree-purge] diff-based purge removing state for ${removed.length} worktree(s):`, removed ) - afterState.purgeWorktreeTerminalState(removed) + const purgeHostId = + options?.executionHostId ?? + (options?.forceLocalOwner ? LOCAL_EXECUTION_HOST_ID : undefined) + afterState.purgeWorktreeTerminalState( + purgeHostId ? removed.map((id) => ({ id, hostId: purgeHostId })) : removed + ) afterState.removeWorkspaceSpaceWorktrees(removed) } } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index c117b08e172..3600bbeb99b 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16234,7 +16234,53 @@ "selectionWithheldOne": "1 selected workspace is hidden by the current filters and was deselected.", "selectionWithheld": "{{value0}} selected workspaces are hidden by the current filters and were deselected.", "selectAllCountOne": "Select 1 safety-checked workspace", - "selectAllCount": "Select all {{value0}} safety-checked workspaces" + "selectAllCount": "Select all {{value0}} safety-checked workspaces", + "appliedFilters": "Applied filters", + "removeFilter": "Remove filter {{value0}}", + "chip": { + "idleDays": "Idle {{value0}}d+", + "neverVisited": "Never visited", + "minSize": "At least {{value0}} MB", + "maxSize": "At most {{value0}} MB", + "excludesUnsized": "Measured only", + "excludesStatusless": "Has a status", + "list": "{{value0}}: {{value1}}", + "triState": "{{value0}}: {{value1}}", + "only": "Only", + "exclude": "Excluded", + "minAhead": "Ahead {{value0}}+", + "minBehind": "Behind {{value0}}+", + "branchQuery": "Branch: {{value0}}", + "pathPrefix": "Path: {{value0}}", + "presence": "{{value0}}: {{value1}}", + "has": "Has", + "none": "None", + "completelyEmpty": "Nothing to lose", + "selectableOnly": "Deletable only", + "kind": { + "status": "Status", + "agent": "Agent", + "git": "Git", + "review": "Review", + "reviewState": "Review state", + "reviewProvider": "Provider", + "ticket": "Ticket", + "ticketSource": "Ticket source", + "context": "Context", + "host": "Host", + "repo": "Repo", + "blocker": "Blocker", + "tier": "Safety", + "dismissed": "Ignored", + "archived": "Archived", + "pinned": "Pinned", + "unread": "Unread", + "comment": "Comment", + "prunable": "Prunable", + "locked": "Locked", + "retainedAgents": "Finished agents" + } + } }, "scan": { "progress": "Scanning workspaces ({{value0}}/{{value1}})", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 2384a7996fc..6164f1707bd 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -14566,6 +14566,54 @@ "presentation": { "gitlabMergeRequestNumber": "MR #{{value0}}", "githubPullRequestNumber": "PR #{{value0}}" + }, + "browse": { + "appliedFilters": "적용된 필터", + "removeFilter": "{{value0}} 필터 제거", + "chip": { + "idleDays": "{{value0}}일 이상 유휴", + "neverVisited": "방문한 적 없음", + "minSize": "{{value0}} MB 이상", + "maxSize": "{{value0}} MB 이하", + "excludesUnsized": "측정된 항목만", + "excludesStatusless": "상태 있음", + "list": "{{value0}}: {{value1}}", + "triState": "{{value0}}: {{value1}}", + "only": "만", + "exclude": "제외", + "minAhead": "{{value0}}개 이상 앞섬", + "minBehind": "{{value0}}개 이상 뒤처짐", + "branchQuery": "브랜치: {{value0}}", + "pathPrefix": "경로: {{value0}}", + "presence": "{{value0}}: {{value1}}", + "has": "있음", + "none": "없음", + "completelyEmpty": "잃을 것 없음", + "selectableOnly": "삭제 가능한 항목만", + "kind": { + "status": "상태", + "agent": "에이전트", + "git": "Git", + "review": "리뷰", + "reviewState": "리뷰 상태", + "reviewProvider": "제공자", + "ticket": "티켓", + "ticketSource": "티켓 소스", + "context": "컨텍스트", + "host": "호스트", + "repo": "저장소", + "blocker": "차단 요인", + "tier": "안전", + "dismissed": "무시됨", + "archived": "보관됨", + "pinned": "고정됨", + "unread": "읽지 않음", + "comment": "댓글", + "prunable": "정리 가능", + "locked": "잠김", + "retainedAgents": "완료된 에이전트" + } + } } } }, diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index fdd53f16ed9..84c05074bb5 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -14557,6 +14557,54 @@ "presentation": { "gitlabMergeRequestNumber": "MR #{{value0}}", "githubPullRequestNumber": "PR #{{value0}}" + }, + "browse": { + "appliedFilters": "已应用的筛选", + "removeFilter": "移除筛选 {{value0}}", + "chip": { + "idleDays": "闲置 {{value0}} 天以上", + "neverVisited": "从未访问", + "minSize": "至少 {{value0}} MB", + "maxSize": "最多 {{value0}} MB", + "excludesUnsized": "仅已测量", + "excludesStatusless": "有状态", + "list": "{{value0}}：{{value1}}", + "triState": "{{value0}}：{{value1}}", + "only": "仅", + "exclude": "排除", + "minAhead": "领先 {{value0}} 个以上", + "minBehind": "落后 {{value0}} 个以上", + "branchQuery": "分支：{{value0}}", + "pathPrefix": "路径：{{value0}}", + "presence": "{{value0}}：{{value1}}", + "has": "有", + "none": "无", + "completelyEmpty": "没有可丢失的内容", + "selectableOnly": "仅可删除", + "kind": { + "status": "状态", + "agent": "代理", + "git": "Git", + "review": "审查", + "reviewState": "审查状态", + "reviewProvider": "提供方", + "ticket": "工单", + "ticketSource": "工单来源", + "context": "上下文", + "host": "主机", + "repo": "仓库", + "blocker": "阻塞项", + "tier": "安全", + "dismissed": "已忽略", + "archived": "已归档", + "pinned": "已固定", + "unread": "未读", + "comment": "评论", + "prunable": "可修剪", + "locked": "已锁定", + "retainedAgents": "已完成的代理" + } + } } } }, diff --git a/src/renderer/src/lib/cmd-j-section-leadership.test.ts b/src/renderer/src/lib/cmd-j-section-leadership.test.ts index 620f32869a5..410bf676e0d 100644 --- a/src/renderer/src/lib/cmd-j-section-leadership.test.ts +++ b/src/renderer/src/lib/cmd-j-section-leadership.test.ts @@ -12,7 +12,7 @@ import type { PaletteDocumentRank } from './palette-match/palette-document' function rank(overrides: Partial = {}): PaletteDocumentRank { return { exactIntent: 1, - matchedDirectField: 0, + containerOnlyTokenCount: 0, wholeQuery: 3, worstQuality: 5, usesSupportingEvidence: 0, diff --git a/src/renderer/src/lib/order-empty-query-worktrees.ts b/src/renderer/src/lib/order-empty-query-worktrees.ts index 08f31534145..54a116d73ce 100644 --- a/src/renderer/src/lib/order-empty-query-worktrees.ts +++ b/src/renderer/src/lib/order-empty-query-worktrees.ts @@ -2,6 +2,7 @@ import type { ExecutionHostId } from '../../../shared/execution-host' import type { Worktree } from '../../../shared/worktree/types' import { isPaletteCurrentWorktree } from './palette-repo-resolution' import { compareWorktreeDisplayName } from './worktree-display-name-order' +import { getWorktreeVisitTimestamp } from './worktree-visit-recency' export type OrderEmptyQueryInputs = { visibleWorktrees: readonly Worktree[] @@ -51,8 +52,8 @@ export function orderEmptyQueryWorktrees(inputs: OrderEmptyQueryInputs): OrderEm // visited SSH worktree below the fold — the exact bug the feature // fixes. Compare presence first, then value within each tier. const sorted = [...switchable].sort((a, b) => { - const aVisited = lastVisitedAtByWorktreeId[a.id] - const bVisited = lastVisitedAtByWorktreeId[b.id] + const aVisited = getWorktreeVisitTimestamp(lastVisitedAtByWorktreeId, a) + const bVisited = getWorktreeVisitTimestamp(lastVisitedAtByWorktreeId, b) if (aVisited != null && bVisited != null) { if (bVisited !== aVisited) { return bVisited - aVisited diff --git a/src/renderer/src/lib/palette-match/match-document.ts b/src/renderer/src/lib/palette-match/match-document.ts index 9d19fe38220..7b2363c1e16 100644 --- a/src/renderer/src/lib/palette-match/match-document.ts +++ b/src/renderer/src/lib/palette-match/match-document.ts @@ -163,8 +163,10 @@ function rankAssignments(args: { let worstQuality: PaletteMatchQuality = 'field-exact' let fuzzyTokenCount = 0 const fields = new Set() - // Derive from field metadata only: an evidence field can itself be a container. - let hasDirectHit = false + let containerOnlyTokenCount = 0 + let tokenIndex = -1 + let tokenHasDirectField = false + let matchedTokenCount = 0 for (const assignment of args.assignments) { if (paletteMatchQualityRank(assignment.quality) > paletteMatchQualityRank(worstQuality)) { @@ -174,20 +176,32 @@ function rankAssignments(args: { fuzzyTokenCount += 1 } fields.add(assignment.fieldId) + if (assignment.tokenIndex !== tokenIndex) { + if (tokenIndex !== -1 && !tokenHasDirectField) { + containerOnlyTokenCount += 1 + } + tokenIndex = assignment.tokenIndex + tokenHasDirectField = false + matchedTokenCount += 1 + } const field = args.document.fieldById.get(assignment.fieldId) if (field && !field.isContainer) { - hasDirectHit = true + tokenHasDirectField = true } } - const isContainerOnly = !hasDirectHit + if (tokenIndex !== -1 && !tokenHasDirectField) { + containerOnlyTokenCount += 1 + } + const isContainerOnly = + containerOnlyTokenCount > 0 && containerOnlyTokenCount === matchedTokenCount return { worstQuality, isContainerOnly, rank: { exactIntent: args.exactIntent ? 0 : 1, - matchedDirectField: isContainerOnly ? 1 : 0, + containerOnlyTokenCount, wholeQuery: args.wholeQuery, worstQuality: paletteMatchQualityRank(worstQuality), usesSupportingEvidence: args.usesEvidence ? 1 : 0, diff --git a/src/renderer/src/lib/palette-match/palette-document.ts b/src/renderer/src/lib/palette-match/palette-document.ts index 2400921f6be..0934ff2cf41 100644 --- a/src/renderer/src/lib/palette-match/palette-document.ts +++ b/src/renderer/src/lib/palette-match/palette-document.ts @@ -129,8 +129,8 @@ export type PaletteSupportingEvidence = { export type PaletteDocumentRank = { /** 0 when a recognized exact intent (such as a task URL) produced this row. */ exactIntent: number - /** 0 when query matched at least one direct field; 1 when all matched tokens are container-only. */ - matchedDirectField: number + /** Tokens whose chosen assignment only matched container fields. */ + containerOnlyTokenCount: number /** 0 equality, 1 prefix, 2 word boundary, 3 none — whole query in visible text. */ wholeQuery: number worstQuality: number @@ -150,7 +150,7 @@ export type PaletteDocumentMatch = { const RANK_KEYS: readonly (keyof PaletteDocumentRank)[] = [ 'exactIntent', - 'matchedDirectField', + 'containerOnlyTokenCount', 'wholeQuery', 'worstQuality', 'usesSupportingEvidence', diff --git a/src/renderer/src/lib/palette-match/palette-match-core.test.ts b/src/renderer/src/lib/palette-match/palette-match-core.test.ts index 8862165bc79..de7bd7696c7 100644 --- a/src/renderer/src/lib/palette-match/palette-match-core.test.ts +++ b/src/renderer/src/lib/palette-match/palette-match-core.test.ts @@ -2,7 +2,11 @@ import { describe, expect, it } from 'vitest' import { matchPaletteDocument } from './match-document' import { mapNormalizedRange, normalizePaletteText } from './normalized-text' import { preparePaletteQuery, PALETTE_QUERY_MAX_TOKENS } from './palette-query' -import { buildPaletteDocument, type PaletteDocumentInput } from './palette-document' +import { + buildPaletteDocument, + comparePaletteDocumentRank, + type PaletteDocumentInput +} from './palette-document' import { segmentPaletteText } from './text-segments' import { isWithinOnePaletteEdit } from './typo-distance' @@ -383,7 +387,7 @@ describe('typo distance', () => { }) describe('container field matching', () => { - it('marks matchedDirectField as 1 and demotes quality class when all tokens land on container fields', () => { + it('counts a container-only token and demotes quality class when every token lands on containers', () => { const tabDoc: PaletteDocumentInput = { id: 'tab-1', visibleFields: [ @@ -394,11 +398,11 @@ describe('container field matching', () => { } const match = run(tabDoc, '4360') expect(match).not.toBeNull() - expect(match?.rank.matchedDirectField).toBe(1) + expect(match?.rank.containerOnlyTokenCount).toBe(1) expect(match?.qualityClass).toBe('exact-evidence') }) - it('marks matchedDirectField as 0 when at least one token lands on a direct field', () => { + it('does not count a token that lands on a direct field', () => { const tabDoc: PaletteDocumentInput = { id: 'tab-1', visibleFields: [ @@ -409,7 +413,40 @@ describe('container field matching', () => { } const match = run(tabDoc, '4360') expect(match).not.toBeNull() - expect(match?.rank.matchedDirectField).toBe(0) + expect(match?.rank.containerOnlyTokenCount).toBe(0) expect(match?.qualityClass).toBe('exact-visible') }) + + it('ranks an all-direct multi-token match ahead of a mixed direct and container match', () => { + const direct = run( + { + id: 'direct', + visibleFields: [ + { id: 'title', profile: 'structured-label', text: 'alpha' }, + { id: 'path', profile: 'structured-label', text: 'beta' } + ], + evidence: [] + }, + 'alpha beta' + ) + const mixed = run( + { + id: 'mixed', + visibleFields: [ + { id: 'title', profile: 'structured-label', text: 'alpha' }, + { id: 'worktree', profile: 'structured-label', text: 'beta', isContainer: true } + ], + evidence: [] + }, + 'alpha beta' + ) + + expect(direct).not.toBeNull() + expect(mixed).not.toBeNull() + expect(direct?.rank.containerOnlyTokenCount).toBe(0) + expect(mixed?.rank.containerOnlyTokenCount).toBe(1) + if (direct && mixed) { + expect(comparePaletteDocumentRank(direct.rank, mixed.rank)).toBeLessThan(0) + } + }) }) diff --git a/src/renderer/src/lib/recent-workspace-tab-rows.test.ts b/src/renderer/src/lib/recent-workspace-tab-rows.test.ts index c36a48dc615..bf822ba3308 100644 --- a/src/renderer/src/lib/recent-workspace-tab-rows.test.ts +++ b/src/renderer/src/lib/recent-workspace-tab-rows.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { buildFocusedGroupTabRecency, + focusedGroupTabKey, orderRecentWorkspaceTabs, resolveRecentWorkspaceTabStatus, type RecentWorkspaceTabRow @@ -130,6 +131,22 @@ describe('orderRecentWorkspaceTabs', () => { ).toEqual(['warm', 'cold']) }) + it('uses host-qualified recency for same-id worktree rows', () => { + const rows = [ + row('local', { worktreeId: 'repo::/app', worktreeHostId: 'local' }), + row('ssh', { worktreeId: 'repo::/app', worktreeHostId: 'ssh:builder' }) + ] + + expect( + order(rows, sources([]), { + lastVisitedAtByWorktreeId: { + 'local|repo::/app': NOW - 1_000, + 'ssh:builder|repo::/app': NOW + } + }) + ).toEqual(['ssh', 'local']) + }) + it('prefers any visited worktree over a never-visited one', () => { const rows = [row('never'), row('ancient')] @@ -151,9 +168,9 @@ describe('orderRecentWorkspaceTabs', () => { order(rows, sources([]), { lastVisitedAtByWorktreeId: { 'wt-1': NOW }, focusedGroupTabRecency: new Map([ - ['unified-first', 0], - ['unified-third', 1], - ['unified-second', 2] + [focusedGroupTabKey('wt-1', 'unified-first'), 0], + [focusedGroupTabKey('wt-1', 'unified-third'), 1], + [focusedGroupTabKey('wt-1', 'unified-second'), 2] ]) }) ).toEqual(['second', 'third', 'first']) @@ -171,14 +188,70 @@ describe('orderRecentWorkspaceTabs', () => { expect( order(rows, sources([]), { focusedGroupTabRecency: new Map([ - ['unified-alpha-1', 0], - ['unified-alpha-2', 1], - ['unified-beta-1', 5] + [focusedGroupTabKey('wt-alpha', 'unified-alpha-1'), 0], + [focusedGroupTabKey('wt-alpha', 'unified-alpha-2'), 1], + [focusedGroupTabKey('wt-beta', 'unified-beta-1'), 5] ]) }) ).toEqual(['alpha-2', 'alpha-1', 'beta-1']) }) + it('keeps an interleaved worktree block together before applying its focused-group MRU', () => { + const rows = [ + row('alpha-old', { worktreeId: 'wt-alpha', unifiedTabId: 'unified-alpha-old' }), + row('beta', { worktreeId: 'wt-beta', unifiedTabId: 'unified-beta' }), + row('alpha-new', { worktreeId: 'wt-alpha', unifiedTabId: 'unified-alpha-new' }) + ] + + expect( + order(rows, sources([]), { + focusedGroupTabRecency: new Map([ + [focusedGroupTabKey('wt-alpha', 'unified-alpha-old'), 0], + [focusedGroupTabKey('wt-alpha', 'unified-alpha-new'), 1], + [focusedGroupTabKey('wt-beta', 'unified-beta'), 0] + ]) + }) + ).toEqual(['alpha-new', 'alpha-old', 'beta']) + }) + + it('keeps duplicate tab ids in separate worktrees on their own MRU ordinals', () => { + const rows = [ + row('alpha-1', { worktreeId: 'wt-alpha', unifiedTabId: 'shared-tab' }), + row('alpha-2', { worktreeId: 'wt-alpha', unifiedTabId: 'alpha-only' }), + row('beta', { worktreeId: 'wt-beta', unifiedTabId: 'shared-tab' }) + ] + + expect( + order(rows, sources([]), { + lastVisitedAtByWorktreeId: { 'wt-alpha': NOW, 'wt-beta': NOW - 1 }, + focusedGroupTabRecency: new Map([ + [focusedGroupTabKey('wt-alpha', 'shared-tab'), 0], + [focusedGroupTabKey('wt-alpha', 'alpha-only'), 1], + // Beta's ordinal for the same tab id must not hoist alpha's occurrence. + [focusedGroupTabKey('wt-beta', 'shared-tab'), 9] + ]) + }) + ).toEqual(['alpha-2', 'alpha-1', 'beta']) + }) + + it('keeps same-id worktrees on two hosts in separate order blocks', () => { + const rows = [ + row('local-old', { worktreeId: 'wt-1', worktreeHostId: 'local', unifiedTabId: 'local-old' }), + row('ssh', { worktreeId: 'wt-1', worktreeHostId: 'ssh:box', unifiedTabId: 'ssh' }), + row('local-new', { worktreeId: 'wt-1', worktreeHostId: 'local', unifiedTabId: 'local-new' }) + ] + + expect( + order(rows, sources([]), { + focusedGroupTabRecency: new Map([ + [focusedGroupTabKey('wt-1', 'local-old'), 0], + [focusedGroupTabKey('wt-1', 'local-new'), 1], + [focusedGroupTabKey('wt-1', 'ssh'), 5] + ]) + }) + ).toEqual(['local-new', 'local-old', 'ssh']) + }) + it('keeps input (positional) order when nothing else separates two rows', () => { const rows = [row('a', { worktreeId: 'wt-1' }), row('b', { worktreeId: 'wt-1' })] @@ -188,6 +261,21 @@ describe('orderRecentWorkspaceTabs', () => { ]) }) + it('returns each occurrence identity when palette ids collide', () => { + const rows = [ + row('workspace-tab:duplicate', { + occurrenceId: 'recent-tab:alpha', + worktreeId: 'wt-alpha' + }), + row('workspace-tab:duplicate', { + occurrenceId: 'recent-tab:beta', + worktreeId: 'wt-beta' + }) + ] + + expect(order(rows, sources([]))).toEqual(['recent-tab:alpha', 'recent-tab:beta']) + }) + it('treats rows without a terminal tab as idle', () => { const rows = [row('browser', { terminalTab: null, unifiedTabId: null }), row('blocked')] @@ -272,8 +360,8 @@ describe('buildFocusedGroupTabRecency', () => { ) expect([...recency]).toEqual([ - ['t1', 0], - ['t2', 1] + [focusedGroupTabKey('wt-1', 't1'), 0], + [focusedGroupTabKey('wt-1', 't2'), 1] ]) }) diff --git a/src/renderer/src/lib/recent-workspace-tab-rows.ts b/src/renderer/src/lib/recent-workspace-tab-rows.ts index 5005030b81a..d17e42eed99 100644 --- a/src/renderer/src/lib/recent-workspace-tab-rows.ts +++ b/src/renderer/src/lib/recent-workspace-tab-rows.ts @@ -10,6 +10,9 @@ import { tabHasLivePty } from './tab-has-live-pty' import type { WorktreeStatus } from './worktree-status' import type { TabGroup } from '../../../shared/tab-types' import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { getWorktreeVisitTimestamp } from './worktree-visit-recency' +import { composeWorktreeHostIdentity } from '../../../shared/worktree/host-qualified-identity' /** * Row model for Cmd+J's empty-query "Recent chats & terminals" section. @@ -20,7 +23,15 @@ import type { TerminalTab } from '../../../shared/terminal-tab-types' export type RecentWorkspaceTabRow = { /** Palette item id. */ id: string + /** + * Per-occurrence identity used while the palette is open. Persisted tab ids can collide across + * hosts (or in a transient duplicate snapshot), so ordering must not use `id` as its key. + * Callers that know their ids are unique may omit this and fall back to `id`. + */ + occurrenceId?: string worktreeId: string + /** Host owner used to distinguish same-id rows published by two hosts. */ + worktreeHostId?: ExecutionHostId /** Unified tab id — the key `TabGroup.recentTabIds` uses. Null for rows outside a tab group. */ unifiedTabId: string | null /** Terminal tab whose panes carry agent state. Null for editor, browser and simulator rows. */ @@ -33,18 +44,19 @@ export type RecentWorkspaceTabOrderInputs = { paneSources: TabPaneInputSources now: number lastVisitedAtByWorktreeId: Record - /** Unified tab id → ordinal in its worktree's focused group; higher is more recent. */ + /** `focusedGroupTabKey` → ordinal in that worktree's focused group; higher is more recent. */ focusedGroupTabRecency: ReadonlyMap } type RankedRow = { - id: string + occurrenceId: string needsAttention: boolean attentionClass: SmartClass attentionTimestamp: number visitedAt: number | undefined focusOrdinal: number worktreeId: string + worktreeOrder: number } /** Classes 1 (blocked/waiting) and 2 (freshly done) are the rows that want the user. */ @@ -89,6 +101,15 @@ export function resolveRecentWorkspaceTabStatus( : 'inactive' } +/** + * Ordinals are per-worktree, so the key must be too: two worktrees can publish the same tab id and + * a bare key would let one overwrite the other's MRU position. + */ +export function focusedGroupTabKey(worktreeId: string, unifiedTabId: string): string { + // NUL separator: a worktree id embeds a filesystem path, so a printable one would be ambiguous. + return `${worktreeId}\u0000${unifiedTabId}` +} + /** `TabGroup.recentTabIds` keeps most-recent at the tail, so the index is the ordinal. */ export function buildFocusedGroupTabRecency( activeGroupIdByWorktree: Record, @@ -102,7 +123,9 @@ export function buildFocusedGroupTabRecency( } // Why: MRU only means something inside the focused group; other groups keep positional order. const focusedGroup = groups?.find((group) => group.id === activeGroupId) - focusedGroup?.recentTabIds?.forEach((tabId, index) => recency.set(tabId, index)) + focusedGroup?.recentTabIds?.forEach((tabId, index) => + recency.set(focusedGroupTabKey(worktreeId, tabId), index) + ) } return recency } @@ -127,10 +150,8 @@ function compareRankedRows(a: RankedRow, b: RankedRow): number { } return b.visitedAt - a.visitedAt } - // Why: focusOrdinal is a per-worktree sequence, so comparing it across worktrees interleaves - // them arbitrarily. Keep input (positional) order instead. - if (a.worktreeId !== b.worktreeId) { - return 0 + if (a.worktreeOrder !== b.worktreeOrder) { + return a.worktreeOrder - b.worktreeOrder } return b.focusOrdinal - a.focusOrdinal } @@ -139,26 +160,41 @@ function compareRankedRows(a: RankedRow, b: RankedRow): number { * Rank rows into ids, most-wanted first: * tier 1 — needs attention: class 1 (blocked/waiting) then 2 (fresh done), newest first * tier 2 — everything else: worktree focus recency, then focused-group MRU - * Full ties keep input order, which callers pass in positional (worktree/group/tab) order. + * Equal worktree tiers preserve first-seen worktree order, then use that worktree's MRU. */ export function orderRecentWorkspaceTabs(inputs: RecentWorkspaceTabOrderInputs): string[] { const { rows, paneSources, now, lastVisitedAtByWorktreeId, focusedGroupTabRecency } = inputs + // Host-qualified: the same worktree id on two hosts is two workspaces and must not share a block. + const worktreeOrder = new Map() + for (const row of rows) { + const identity = composeWorktreeHostIdentity(row.worktreeHostId, row.worktreeId) + if (!worktreeOrder.has(identity)) { + worktreeOrder.set(identity, worktreeOrder.size) + } + } return rows .map((row): RankedRow => { const attention = resolveRecentWorkspaceTabAttention(row, paneSources, now) return { - id: row.id, + occurrenceId: row.occurrenceId ?? row.id, needsAttention: attention.cls <= NEEDS_ATTENTION_MAX_CLASS, attentionClass: attention.cls, attentionTimestamp: attention.attentionTimestamp, - visitedAt: lastVisitedAtByWorktreeId[row.worktreeId], + visitedAt: getWorktreeVisitTimestamp(lastVisitedAtByWorktreeId, { + id: row.worktreeId, + hostId: row.worktreeHostId + }), worktreeId: row.worktreeId, + worktreeOrder: + worktreeOrder.get(composeWorktreeHostIdentity(row.worktreeHostId, row.worktreeId)) ?? + Number.MAX_SAFE_INTEGER, focusOrdinal: row.unifiedTabId === null ? NO_FOCUS_ORDINAL - : (focusedGroupTabRecency.get(row.unifiedTabId) ?? NO_FOCUS_ORDINAL) + : (focusedGroupTabRecency.get(focusedGroupTabKey(row.worktreeId, row.unifiedTabId)) ?? + NO_FOCUS_ORDINAL) } }) .sort(compareRankedRows) - .map((row) => row.id) + .map((row) => row.occurrenceId) } diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index b84b50909f7..9d8cdb34857 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -11,6 +11,10 @@ import { } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' +import { + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' import { mergeWorkspaceSessionsFromHosts, splitWorkspaceSessionByHost, @@ -76,6 +80,9 @@ function isPlainRecord(value: unknown): value is Record { } function normalizeWorkspaceSessionKeyForOwnerMap(value: string): string { + if (isWorktreeHostIdentity(value)) { + return getWorktreeIdFromHostIdentity(value) + } const scope = parseWorkspaceKey(value) return scope?.type === 'worktree' ? scope.worktreeId : value } diff --git a/src/renderer/src/lib/workspace-session-host-split.test.ts b/src/renderer/src/lib/workspace-session-host-split.test.ts index a3aff69a888..de359cfb12e 100644 --- a/src/renderer/src/lib/workspace-session-host-split.test.ts +++ b/src/renderer/src/lib/workspace-session-host-split.test.ts @@ -113,6 +113,32 @@ describe('splitWorkspaceSessionByHost', () => { expect(Object.keys(slices[RUNTIME_B]?.tabsByWorktree ?? {})).toEqual(['b-wt']) }) + it('keeps ssh-qualified visit recency in the local slice and routes runtime-qualified keys to their partition', () => { + const state: WorkspaceSessionState = { + ...getDefaultWorkspaceSession(), + lastVisitedAtByWorktreeId: { + 'local-wt': 1, + 'a-wt': 2, + 'ssh:builder|ssh-wt': 3, + 'runtime:env-a|a-wt': 4 + } + } + + const slices = splitWorkspaceSessionByHost(state, ownerByPrefix()) + + // Why local for ssh: boot hydration reads only local + runtime:* partitions, + // so an ssh partition would strand the recency across restarts. + expect(slices[LOCAL_EXECUTION_HOST_ID]?.lastVisitedAtByWorktreeId).toEqual({ + 'local-wt': 1, + 'ssh:builder|ssh-wt': 3 + }) + expect(slices[RUNTIME_A]?.lastVisitedAtByWorktreeId).toEqual({ + 'a-wt': 2, + 'runtime:env-a|a-wt': 4 + }) + expect(slices['ssh:builder' as ExecutionHostId]).toBeUndefined() + }) + it('routes tab-keyed maps via the owning tab worktree (legacy + unified)', () => { const state: WorkspaceSessionState = { ...getDefaultWorkspaceSession(), diff --git a/src/renderer/src/lib/workspace-session-host-split.ts b/src/renderer/src/lib/workspace-session-host-split.ts index 103a1abcf63..ab374474b73 100644 --- a/src/renderer/src/lib/workspace-session-host-split.ts +++ b/src/renderer/src/lib/workspace-session-host-split.ts @@ -1,5 +1,10 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' -import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/execution-host' +import { + LOCAL_EXECUTION_HOST_ID, + parseExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' +import { isWorktreeHostIdentity } from '../../../shared/worktree/host-qualified-identity' import { GLOBAL_WORKSPACE_SESSION_FIELDS, WORKSPACE_SESSION_FIELD_OWNERSHIP @@ -84,6 +89,33 @@ function assignWorktreeKeyed( } } +function assignVisitRecencyByHost( + slices: HostSessionSlices, + template: WorkspaceSessionState, + value: unknown, + ctx: SplitContext +): void { + if (!isWorkspaceSessionRecord(value)) { + return + } + for (const [key, entry] of Object.entries(value)) { + // Why: boot hydration reads only local + runtime:* partitions, and SSH worktree + // session state deliberately stays in the local partition (see buildHostIdByWorktreeId); + // routing ssh-qualified keys to an ssh partition would strand them across restarts. + const qualifiedHost = isWorktreeHostIdentity(key) + ? parseExecutionHostId(key.slice(0, key.indexOf('|'))) + : null + const host = isWorktreeHostIdentity(key) + ? qualifiedHost?.kind === 'runtime' + ? qualifiedHost.id + : LOCAL_EXECUTION_HOST_ID + : ctx.hostIdByWorktreeId(key) + const slice = ensureSlice(slices, host, template) as WorkspaceSessionRecord + const target = (slice.lastVisitedAtByWorktreeId ??= {}) as WorkspaceSessionRecord + target[key] = entry + } +} + function assignKeyedByResolvedWorktree( slices: HostSessionSlices, template: WorkspaceSessionState, @@ -157,7 +189,11 @@ export function splitWorkspaceSessionByHost( // Main preserves this field while rebasing renderer writes onto host authority. break case 'worktreeKeyed': - assignWorktreeKeyed(slices, template, field, value, ctx) + if (field === 'lastVisitedAtByWorktreeId') { + assignVisitRecencyByHost(slices, template, value, ctx) + } else { + assignWorktreeKeyed(slices, template, field, value, ctx) + } break case 'worktreeArray': { if (!Array.isArray(value)) { diff --git a/src/renderer/src/lib/workspace-tab-agent-snippet-match.ts b/src/renderer/src/lib/workspace-tab-agent-snippet-match.ts index 3a67ed6a11b..ae8b0d497c2 100644 --- a/src/renderer/src/lib/workspace-tab-agent-snippet-match.ts +++ b/src/renderer/src/lib/workspace-tab-agent-snippet-match.ts @@ -21,7 +21,7 @@ import type { AgentMetadata } from './workspace-tab-agent-metadata' */ const AGENT_SNIPPET_RANK: PaletteDocumentRank = { exactIntent: 1, - matchedDirectField: 0, + containerOnlyTokenCount: Number.MAX_SAFE_INTEGER, wholeQuery: 3, worstQuality: paletteMatchQualityRank(PALETTE_MATCH_QUALITIES.at(-1) as PaletteMatchQuality) + 1, usesSupportingEvidence: 1, diff --git a/src/renderer/src/lib/workspace-tab-palette-results.test.ts b/src/renderer/src/lib/workspace-tab-palette-results.test.ts index a62c6870ae8..8c34265d3a1 100644 --- a/src/renderer/src/lib/workspace-tab-palette-results.test.ts +++ b/src/renderer/src/lib/workspace-tab-palette-results.test.ts @@ -99,13 +99,14 @@ function makeEntry({ describe('searchWorkspaceTabs lastActiveAt', () => { it('is null when neither agent activity nor worktree activity is known', () => { - const [result] = searchWorkspaceTabs([makeEntry()], '') + const [result] = searchWorkspaceTabs([makeEntry({ createdAt: 4000 })], '') expect(result.lastActiveAt).toBeNull() }) it('falls back to worktree PTY activity for editor tabs with no agent metadata', () => { const entry = makeEntry({ contentType: 'editor', + createdAt: 1000, worktree: makeWorktree({ lastActivityAt: 5000 }) }) const [result] = searchWorkspaceTabs([entry], '') @@ -148,9 +149,61 @@ describe('searchWorkspaceTabs lastActiveAt', () => { const [result] = searchWorkspaceTabs([entry], '') expect(result.lastActiveAt).toBe(6000) }) + + it('uses a newer tab focus instead of stale agent activity', () => { + const entry = makeEntry({ + id: 'tab-refocused', + createdAt: 1000, + agentLastActivityAt: 3000 + }) + entry.tab.lastFocusedAt = 8000 + + const [result] = searchWorkspaceTabs([entry], '') + + expect(result.lastActiveAt).toBe(8000) + }) + + it('uses newer agent activity instead of an older tab focus', () => { + const entry = makeEntry({ + id: 'tab-agent-active', + createdAt: 1000, + agentLastActivityAt: 8000 + }) + entry.tab.lastFocusedAt = 3000 + + const [result] = searchWorkspaceTabs([entry], '') + + expect(result.lastActiveAt).toBe(8000) + }) }) describe('searchWorkspaceTabs ranking', () => { + it('ranks a multi-token direct-plus-container hit above a container-only whole-query hit', () => { + const directEntry = makeEntry({ id: 'direct-tab' }) + const containerEntry = makeEntry({ id: 'container-tab' }) + directEntry.document = buildPaletteTabDocument({ + id: 'direct', + title: 'Auth notes', + secondaryTexts: [], + worktreeName: 'Aurora migration', + branch: 'main', + repoName: 'repo' + }) + containerEntry.document = buildPaletteTabDocument({ + id: 'container', + title: 'Unrelated notes', + secondaryTexts: [], + worktreeName: 'Auth Aurora', + branch: 'main', + repoName: 'repo' + }) + + const results = searchWorkspaceTabs([containerEntry, directEntry], 'auth aurora') + + expect(results.map((result) => result.tabId)).toEqual(['direct-tab', 'container-tab']) + expect(results.map((result) => result.rank?.containerOnlyTokenCount)).toEqual([1, 2]) + }) + it('ranks direct tab title matches ahead of container-only worktree matches', () => { const directEntry = makeEntry({ id: 'README-4360' }) const containerEntry = makeEntry({ id: 'unrelated-file' }) @@ -175,9 +228,9 @@ describe('searchWorkspaceTabs ranking', () => { const results = searchWorkspaceTabs([containerEntry, directEntry], '4360') expect(results).toHaveLength(2) expect(results[0].tabId).toBe('README-4360') - expect(results[0].rank?.matchedDirectField).toBe(0) + expect(results[0].rank?.containerOnlyTokenCount).toBe(0) expect(results[1].tabId).toBe('unrelated-file') - expect(results[1].rank?.matchedDirectField).toBe(1) + expect(results[1].rank?.containerOnlyTokenCount).toBe(1) }) it('breaks tie between two container-matching tabs using lastActiveAt recency', () => { diff --git a/src/renderer/src/lib/workspace-tab-palette-results.ts b/src/renderer/src/lib/workspace-tab-palette-results.ts index 67d77bec921..9eeeff525f6 100644 --- a/src/renderer/src/lib/workspace-tab-palette-results.ts +++ b/src/renderer/src/lib/workspace-tab-palette-results.ts @@ -87,15 +87,13 @@ function positionScore(entry: SearchableWorkspaceTab): number { } function resolveWorkspaceTabLastActiveAt(entry: SearchableWorkspaceTab): number | null { - // Agent activity wins when present; then explicit tab focus; otherwise fall back to worktree. - const candidate = - maxAgentActivityAt(entry.agentMetadata) ?? - entry.tab.lastFocusedAt ?? - (entry.worktree.lastActivityAt || null) + // Why: explicit tab activity outranks the worktree fallback; creation only clamps stale signals. + const tabLocalActivity = + Math.max(maxAgentActivityAt(entry.agentMetadata) ?? 0, entry.tab.lastFocusedAt ?? 0) || null + const candidate = tabLocalActivity || entry.worktree.lastActivityAt || null if (candidate == null) { return null } - // Never report activity older than the tab itself (e.g. a stale worktree signal). return Math.max(candidate, entry.tab.createdAt) } diff --git a/src/renderer/src/lib/worktree-palette-task-url-result.ts b/src/renderer/src/lib/worktree-palette-task-url-result.ts index 303083dff92..27f06757387 100644 --- a/src/renderer/src/lib/worktree-palette-task-url-result.ts +++ b/src/renderer/src/lib/worktree-palette-task-url-result.ts @@ -38,7 +38,7 @@ export function buildWorktreePaletteTaskUrlResult(args: { qualityClass: 'exact-intent', rank: { exactIntent: 0, - matchedDirectField: 0, + containerOnlyTokenCount: 0, wholeQuery: 0, worstQuality: 0, usesSupportingEvidence: 1, diff --git a/src/renderer/src/lib/worktree-visit-recency.test.ts b/src/renderer/src/lib/worktree-visit-recency.test.ts new file mode 100644 index 00000000000..4b431886a1a --- /dev/null +++ b/src/renderer/src/lib/worktree-visit-recency.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' +import { + getWorktreeVisitKey, + getWorktreeVisitTimestamp, + removeWorktreeVisitEntries +} from './worktree-visit-recency' + +describe('worktree visit recency identities', () => { + it('keeps same-id host twins independent when reading and writing keys', () => { + const id = 'repo::/srv/app' + const localKey = getWorktreeVisitKey(id, 'local') + const sshKey = getWorktreeVisitKey(id, 'ssh:builder') + const timestamps = { [localKey]: 100, [sshKey]: 200 } + + expect(localKey).toBe(`local|${id}`) + expect(sshKey).toBe(`ssh:builder|${id}`) + expect(getWorktreeVisitTimestamp(timestamps, { id, hostId: 'local' })).toBe(100) + expect(getWorktreeVisitTimestamp(timestamps, { id, hostId: 'ssh:builder' })).toBe(200) + }) + + it('falls back to a legacy bare key only when the host key is absent', () => { + const id = 'repo::/srv/app' + expect(getWorktreeVisitTimestamp({ [id]: 100 }, { id, hostId: 'ssh:builder' })).toBe(100) + expect( + getWorktreeVisitTimestamp( + { [id]: 100, [getWorktreeVisitKey(id, 'ssh:builder')]: 200 }, + { id, hostId: 'ssh:builder' } + ) + ).toBe(200) + }) + + it('removes one host key without deleting the other host twin or legacy fallback', () => { + const id = 'repo::/srv/app' + const timestamps = { + [id]: 50, + [getWorktreeVisitKey(id, 'local')]: 100, + [getWorktreeVisitKey(id, 'ssh:builder')]: 200 + } + expect(removeWorktreeVisitEntries(timestamps, new Set([id]), 'local')).toEqual({ + [id]: 50, + [getWorktreeVisitKey(id, 'ssh:builder')]: 200 + }) + }) +}) diff --git a/src/renderer/src/lib/worktree-visit-recency.ts b/src/renderer/src/lib/worktree-visit-recency.ts new file mode 100644 index 00000000000..0fa9533ce33 --- /dev/null +++ b/src/renderer/src/lib/worktree-visit-recency.ts @@ -0,0 +1,103 @@ +import type { ExecutionHostId } from '../../../shared/execution-host' +import { + composeWorktreeHostIdentity, + getWorktreeHostIdentity, + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from '../../../shared/worktree/host-qualified-identity' +import type { Worktree } from '../../../shared/worktree/types' + +/** The persisted focus-recency map is host-qualified for new writes. */ +export type WorktreeVisitOwner = Pick + +/** + * Read recency for one visible row. + * + * The bare lookup is deliberately second: it keeps sessions written before + * host-qualified recency useful without letting a legacy value override a + * newer host-specific visit. + */ +export function getWorktreeVisitTimestamp( + timestamps: Readonly> | undefined, + worktree: WorktreeVisitOwner +): number | undefined { + if (!timestamps) { + return undefined + } + const qualified = timestamps[getWorktreeHostIdentity(worktree)] + return qualified ?? timestamps[worktree.id] +} + +/** + * Key used for a newly stamped visit. Unknown-host rows keep the legacy bare + * key because there is no safe host qualification to persist; once a row has + * host evidence, all new writes use the canonical `${host}|${id}` form. + */ +export function getWorktreeVisitKey(worktreeId: string, hostId?: ExecutionHostId): string { + return hostId ? composeWorktreeHostIdentity(hostId, worktreeId) : worktreeId +} + +/** + * Decode only keys that use the canonical host-qualified shape. Bare legacy + * ids are returned unchanged, including ids that happen to contain `|`. + */ +export function getWorktreeIdFromVisitKey(key: string): string { + const separator = key.indexOf('|') + if (separator === -1) { + return key + } + // Empty host is the canonical unknown-host bucket. Known host ids are the + // other valid prefixes; a legacy worktree id may itself contain `|`. + if (isWorktreeHostIdentity(key)) { + return getWorktreeIdFromHostIdentity(key) + } + return key +} + +export function isHostQualifiedVisitKey(key: string): boolean { + return getWorktreeIdFromVisitKey(key) !== key +} + +/** Remove all recency entries belonging to one or more raw worktree ids. */ +export function removeWorktreeVisitEntries( + timestamps: Readonly>, + worktreeIds: ReadonlySet, + hostId?: ExecutionHostId +): Record { + return removeWorktreeVisitEntriesForTargets( + timestamps, + [...worktreeIds].map((id) => ({ id, hostId })) + ) +} + +/** + * Remove recency for bulk purge targets. A host-qualified target only removes + * that host's key; an unknown-host target deliberately retains the legacy + * raw-id behavior because no owner can safely be inferred. + */ +export function removeWorktreeVisitEntriesForTargets( + timestamps: Readonly>, + targets: readonly WorktreeVisitOwner[] +): Record { + let changed = false + const next: Record = {} + const qualified = new Set() + const legacyIds = new Set() + for (const target of targets) { + if (target.hostId) { + qualified.add(getWorktreeVisitKey(target.id, target.hostId)) + } else { + legacyIds.add(target.id) + } + } + for (const [key, timestamp] of Object.entries(timestamps)) { + const rawId = getWorktreeIdFromVisitKey(key) + const remove = qualified.has(key) || legacyIds.has(rawId) + if (remove) { + changed = true + continue + } + next[key] = timestamp + } + return changed ? next : (timestamps as Record) +} diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index 95cde4a3979..756e91549fc 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -2343,6 +2343,15 @@ export const createAgentStatusSlice: StateCreator { ] }, tabsByWorktree: { [worktreeId]: tabs }, + lastVisitedAtByWorktreeId: { + [worktreeId]: 50, + [`${RUNTIME_A}|${worktreeId}`]: 100, + [`${RUNTIME_B}|${worktreeId}`]: 200 + }, restoredRuntimeHostIdByWorkspaceSessionKey: { [worktreeId]: RUNTIME_B }, activeWorktreeId: worktreeId, activeWorkspaceKey: worktreeWorkspaceKey(worktreeId) @@ -269,6 +274,10 @@ describe('purgeStaleRuntimeHostState', () => { expect(s.worktreesByRepo.shared).toHaveLength(1) expect(s.worktreesByRepo.shared[0]?.hostId).toBe(RUNTIME_B) expect(s.tabsByWorktree[worktreeId]).toBe(tabs) + expect(s.lastVisitedAtByWorktreeId).toEqual({ + [worktreeId]: 50, + [`${RUNTIME_B}|${worktreeId}`]: 200 + }) expect(s.restoredRuntimeHostIdByWorkspaceSessionKey).toBe(restoredOwners) expect(s.restoredRuntimeHostIdByWorkspaceSessionKey[worktreeId]).toBe(RUNTIME_B) expect(s.activeWorktreeId).toBe(worktreeId) diff --git a/src/renderer/src/store/slices/repos-host-identity-routing.test.ts b/src/renderer/src/store/slices/repos-host-identity-routing.test.ts index 5eba51545d4..1c8cd78be57 100644 --- a/src/renderer/src/store/slices/repos-host-identity-routing.test.ts +++ b/src/renderer/src/store/slices/repos-host-identity-routing.test.ts @@ -366,6 +366,31 @@ describe('repo slice host identity routing', () => { expect(ptyKill).not.toHaveBeenCalledWith('remote-pty') }) + it('preserves qualified recency for an exact-id sibling host', async () => { + const worktreeId = 'same-repo::/shared/wt' + const localWorktree = makeWorktree({ id: worktreeId, repoId: 'same-repo' }) + const remoteWorktree = makeWorktree({ + id: worktreeId, + repoId: 'same-repo', + hostId: 'runtime:env-1' + }) + const store = createTestStore() + store.setState({ + repos: [localDuplicate, remoteDuplicate], + worktreesByRepo: { 'same-repo': [localWorktree, remoteWorktree] }, + lastVisitedAtByWorktreeId: { + [`local|${worktreeId}`]: 10, + [`runtime:env-1|${worktreeId}`]: 20 + } + }) + + await store.getState().removeProject('same-repo', { hostId: 'local' }) + + expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ + [`runtime:env-1|${worktreeId}`]: 20 + }) + }) + it('removeProject with an explicit hostId routes to that host, not the focused one', async () => { runtimeEnvironmentCall.mockResolvedValue({ id: 'rpc-explicit-host', diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 91cb7655790..56bd44159e6 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -50,6 +50,7 @@ import { applyManualRepoOrder, getManualRepoOrder } from '../../../../shared/man import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups' import { isPathInsideOrEqual } from '../../../../shared/cross-platform-path' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' +import { getWorktreeIdFromVisitKey, getWorktreeVisitKey } from '@/lib/worktree-visit-recency' import { structuralValuesEqual } from '../../../../shared/structural-value-equality' import { selectProjectGroupRemovalTargets } from './project-group-removal-targets' import { @@ -3700,6 +3701,20 @@ export const createRepoSlice: StateCreator = (set, // Kill PTYs for all worktrees belonging to this repo const worktreeIds = getKnownRepoWorktreeIds(get(), projectId, ownerHostId) + // A raw id can be published by two hosts. Keep the purge host-scoped for + // those twins so the sibling's qualified visit recency survives. + const knownRepoWorktrees = [ + ...(get().worktreesByRepo[projectId] ?? []), + ...(get().detectedWorktreesByRepo[projectId]?.worktrees ?? []) + ] + const exactSiblingIds = new Set( + knownRepoWorktrees + .filter((worktree) => !worktreeBelongsToHost(worktree, ownerHostId)) + .map((worktree) => worktree.id) + ) + const purgeTargets = worktreeIds.map((id) => + exactSiblingIds.has(id) ? { id, hostId: ownerHostId } : id + ) const localAgentContextProjectIds = ownerHostId === LOCAL_EXECUTION_HOST_ID ? [ @@ -3735,7 +3750,7 @@ export const createRepoSlice: StateCreator = (set, } // Why: use the canonical per-worktree purge to evict all worktree-scoped maps (hand-deletion leaked most); runs before the set() below so it still sees tabsByWorktree. - get().purgeWorktreeTerminalState(worktreeIds) + get().purgeWorktreeTerminalState(purgeTargets) get().clearLocalDetectedAgentContextsForProjects(localAgentContextProjectIds) set((s) => { @@ -3774,6 +3789,9 @@ export const createRepoSlice: StateCreator = (set, } // Why: editor state is worktree-scoped; clear the repo's open files + active-file tracking so orphans don't linger in the session save. const worktreeIdSet = new Set(worktreeIds) + const removedVisitKeys = new Set( + worktreeIds.map((worktreeId) => getWorktreeVisitKey(worktreeId, ownerHostId)) + ) const nextOpenFiles = s.openFiles.filter((f) => !worktreeIdSet.has(f.worktreeId)) const nextActiveFileIdByWorktree = { ...s.activeFileIdByWorktree } const nextActiveTabTypeByWorktree = { ...s.activeTabTypeByWorktree } @@ -3789,9 +3807,11 @@ export const createRepoSlice: StateCreator = (set, const repoIdFullyRemoved = !nextRepos.some((r) => r.id === projectId) let nextLastVisitedAtByWorktreeId = s.lastVisitedAtByWorktreeId for (const id of Object.keys(s.lastVisitedAtByWorktreeId)) { + const rawId = getWorktreeIdFromVisitKey(id) if ( - worktreeIdSet.has(id) || - (repoIdFullyRemoved && getRepoIdFromWorktreeId(id) === projectId) + (ownerHostId && removedVisitKeys.has(id)) || + (!ownerHostId && worktreeIdSet.has(rawId)) || + (repoIdFullyRemoved && getRepoIdFromWorktreeId(rawId) === projectId) ) { if (nextLastVisitedAtByWorktreeId === s.lastVisitedAtByWorktreeId) { nextLastVisitedAtByWorktreeId = { ...s.lastVisitedAtByWorktreeId } diff --git a/src/renderer/src/store/slices/workspace-cleanup.ts b/src/renderer/src/store/slices/workspace-cleanup.ts index b93fa13571e..063bd9d1d4f 100644 --- a/src/renderer/src/store/slices/workspace-cleanup.ts +++ b/src/renderer/src/store/slices/workspace-cleanup.ts @@ -43,6 +43,7 @@ import { throwIfWorkspaceCleanupScanSuperseded } from './workspace-cleanup-broad-scan-registry' import { classifyTitleActivity, isExplicitAgentStatusFresh } from '@/lib/pane-agent-evidence' +import { getWorktreeVisitTimestamp } from '@/lib/worktree-visit-recency' import type { PreservedBranchCleanup } from '@/lib/preserved-branch-cleanup' export type WorkspaceCleanupFailure = { @@ -821,7 +822,12 @@ function getInitialWorkspaceCleanupGitDeferrals(state: AppState): string[] { const hasVisibleContext = openEditorWorktreeIds.has(worktreeId) || (state.browserTabsByWorktree[worktreeId]?.length ?? 0) > 0 - const lastVisitedAt = state.lastVisitedAtByWorktreeId[worktreeId] ?? 0 + // Why: enrichment state may be a plain snapshot without slice methods. + const lastVisitedAt = + getWorktreeVisitTimestamp(state.lastVisitedAtByWorktreeId, { + id: worktreeId, + hostId: state.getKnownWorktreeById?.(worktreeId)?.hostId + }) ?? 0 if ( hasVisibleContext && lastVisitedAt > 0 && @@ -991,7 +997,11 @@ function getWorkspaceCleanupLocalStateSignature( browserTabCount: (state.browserTabsByWorktree[worktreeId] ?? []).length, retainedDoneAgentPaneKeys, agentStatuses, - lastVisitedAt: state.lastVisitedAtByWorktreeId[worktreeId] ?? 0, + lastVisitedAt: + getWorktreeVisitTimestamp(state.lastVisitedAtByWorktreeId, { + id: worktreeId, + hostId: getWorkspaceCleanupCandidateHostId(candidate) + }) ?? 0, viewed: state.workspaceCleanupViewedCandidates[worktreeId] ?? null, dismissal }) @@ -1036,7 +1046,11 @@ async function enrichWorkspaceCleanupCandidate( blockers.push('terminal-liveness-unknown') } - const lastVisitedAt = state.lastVisitedAtByWorktreeId[candidate.worktreeId] ?? 0 + const lastVisitedAt = + getWorktreeVisitTimestamp(state.lastVisitedAtByWorktreeId, { + id: candidate.worktreeId, + hostId: getWorkspaceCleanupCandidateHostId(candidate) + }) ?? 0 const hasVisibleContext = cleanEditorTabCount > 0 || browserTabCount > 0 if ( hasVisibleContext && diff --git a/src/renderer/src/store/slices/worktree-delete-state-types.ts b/src/renderer/src/store/slices/worktree-delete-state-types.ts new file mode 100644 index 00000000000..85178bc15dc --- /dev/null +++ b/src/renderer/src/store/slices/worktree-delete-state-types.ts @@ -0,0 +1,15 @@ +import type { ExecutionHostId } from '../../../../shared/execution-host' +import type { WorktreeForceDeleteReason } from '../../../../shared/worktree/removal' +import type { Worktree } from '../../../../shared/worktree/types' + +export type WorktreeDeleteState = { + isDeleting: boolean + phase?: 'deleting' | 'queued' + executionHostId?: ExecutionHostId | null + error: string | null + canForceDelete: boolean + forceDeleteReason: WorktreeForceDeleteReason | null + lockReason?: string | null +} + +export type WorktreeDeleteStateTarget = Pick diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index a67bcfdfd6b..fb368bec070 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -24,10 +24,7 @@ import type { Worktree } from '../../../../shared/worktree/types' import type { TaskSourceContext } from '../../../../shared/task-source-context' -import type { - WorktreeForceDeleteReason, - WorktreeRemovalTarget -} from '../../../../shared/worktree/removal' +import type { WorktreeRemovalTarget } from '../../../../shared/worktree/removal' import type { TerminalGitHubPRLink } from '../../../../shared/terminal-github-pr-link-detector' import type { ExecutionHostId } from '../../../../shared/execution-host' import type { RemoveWorktreeOptions } from './worktree-removal-options' @@ -43,20 +40,12 @@ import type { import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' import type { AppState } from '../types' import type { WorktreeRefreshAllOptions } from './worktree-refresh-options' +export type { WorktreePurgeTarget, WorktreePurgeTargets } from './worktree-purge-target' +import type { WorktreePurgeTargets } from './worktree-purge-target' +export type { WorktreeDeleteState, WorktreeDeleteStateTarget } from './worktree-delete-state-types' +import type { WorktreeDeleteState, WorktreeDeleteStateTarget } from './worktree-delete-state-types' export { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' -export type WorktreeDeleteState = { - isDeleting: boolean - phase?: 'deleting' | 'queued' - executionHostId?: ExecutionHostId | null - error: string | null - canForceDelete: boolean - forceDeleteReason: WorktreeForceDeleteReason | null - lockReason?: string | null -} - -export type WorktreeDeleteStateTarget = Pick - import type { RendererRemoveWorktreeResult } from './renderer-remove-worktree-result' export type WorktreeFetchOptions = { @@ -147,6 +136,7 @@ export type WorktreeSlice = { * (activateAndRevealWorktree), NOT from background activity events or raw * `setActiveWorktree` calls. See docs/cmd-j-empty-query-ordering.md. */ + /** New host-qualified rows use `${host}|${worktreeId}`; legacy bare ids remain readable. */ lastVisitedAtByWorktreeId: Record /** * Guards the one-shot hydration-time purge in `fetchAllWorktrees`. Set to @@ -296,7 +286,11 @@ export type WorktreeSlice = { * stored value. Called from user-initiated activations only. See * docs/cmd-j-empty-query-ordering.md. */ - markWorktreeVisited: (worktreeId: string, visitedAt?: number) => void + markWorktreeVisited: ( + worktreeId: string, + visitedAt?: number, + executionHostId?: ExecutionHostId + ) => void /** * Drop `lastVisitedAtByWorktreeId` entries whose worktree IDs no longer * exist. Must be called AFTER worktree hydration completes — repos load @@ -337,7 +331,7 @@ export type WorktreeSlice = { * Called by the `worktrees:changed` listener on server-side deletions and * one-shot at hydration time. See design §4.4. */ - purgeWorktreeTerminalState: (worktreeIds: string[]) => void + purgeWorktreeTerminalState: (worktreeTargets: WorktreePurgeTargets) => void /** * Retires every client-store row (repos, project host setups, worktree + * detected-worktree rows, and their tab/PTY/browser/editor cascade) owned by a diff --git a/src/renderer/src/store/slices/worktree-purge-target.ts b/src/renderer/src/store/slices/worktree-purge-target.ts new file mode 100644 index 00000000000..031765acccf --- /dev/null +++ b/src/renderer/src/store/slices/worktree-purge-target.ts @@ -0,0 +1,7 @@ +import type { Worktree } from '../../../../shared/worktree/types' + +/** A purge target with optional host ownership for host-scoped state. */ +export type WorktreePurgeTarget = Pick + +/** Strings remain supported for callers that only have a raw worktree id. */ +export type WorktreePurgeTargets = readonly (string | WorktreePurgeTarget)[] diff --git a/src/renderer/src/store/slices/worktrees-workspace-selection-state.test.ts b/src/renderer/src/store/slices/worktrees-workspace-selection-state.test.ts index c2519d6909a..b845b76e594 100644 --- a/src/renderer/src/store/slices/worktrees-workspace-selection-state.test.ts +++ b/src/renderer/src/store/slices/worktrees-workspace-selection-state.test.ts @@ -115,6 +115,18 @@ describe('setActiveWorktree focus handling', () => { }) describe('markWorktreeVisited', () => { + it('stamps a host-qualified key when the worktree owner is known', () => { + const store = createTestStore() + const worktree = makeWorktree({ id: 'repo1::/hosted', repoId: 'repo1', hostId: 'ssh:builder' }) + store.setState({ worktreesByRepo: { repo1: [worktree] } } as Partial) + + store.getState().markWorktreeVisited(worktree.id, 1000) + + expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ + [`ssh:builder|${worktree.id}`]: 1000 + }) + }) + it('is monotonic: an older timestamp does not regress the stored value', () => { const store = createTestStore() store.getState().markWorktreeVisited('wt-1', 1000) @@ -155,6 +167,28 @@ describe('markWorktreeVisited', () => { expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ 'repo1::/a': 100 }) }) + it('prunes host-qualified recency by host while retaining a same-id twin', () => { + const store = createTestStore() + const id = 'repo1::/shared' + const local = makeWorktree({ id, repoId: 'repo1', path: '/shared', hostId: 'local' }) + const ssh = makeWorktree({ id, repoId: 'repo1', path: '/shared', hostId: 'ssh:builder' }) + store.setState({ + worktreesByRepo: { repo1: [local, ssh] }, + lastVisitedAtByWorktreeId: { + [`local|${id}`]: 100, + [`ssh:builder|${id}`]: 200, + 'local|repo1::/gone': 300 + } + } as Partial) + + store.getState().pruneLastVisitedTimestamps() + + expect(store.getState().lastVisitedAtByWorktreeId).toEqual({ + [`local|${id}`]: 100, + [`ssh:builder|${id}`]: 200 + }) + }) + it('pruneLastVisitedTimestamps preserves entries for not-yet-hydrated repos (e.g. SSH pre-connect)', () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/a', repoId: 'repo1', path: '/a' }) diff --git a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts index 2311aaf0294..46b4f2fa6b4 100644 --- a/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts +++ b/src/renderer/src/store/slices/worktrees/listing/fetched-worktree-merge.ts @@ -162,7 +162,12 @@ export function mergeFetchedWorktrees( } } : {}), - ...(removedIds.length > 0 ? buildWorktreePurgeState(s, removedIds) : {}) + ...(removedIds.length > 0 + ? buildWorktreePurgeState( + s, + removedIds.map((id) => ({ id, hostId: args.hostId })) + ) + : {}) } }) if (admitted) { diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts b/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts index b2b25992d7a..1a841fbdfc9 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-identity-rename-state.ts @@ -1,6 +1,7 @@ import type { AppState } from '../../../types' import { splitWorktreeIdForFilesystem } from '../../../../../../shared/worktree/id' import { worktreeWorkspaceKey } from '../../../../../../shared/workspace-scope' +import { getWorktreeIdFromVisitKey } from '@/lib/worktree-visit-recency' import { remapClosedTerminalTabSnapshotCwds, type ClosedTerminalTabSnapshot @@ -41,7 +42,6 @@ const WORKTREE_ID_KEYED_MAP_KEYS = [ 'gitBranchCompareRequestStatusHeadByWorktree', 'showDotfilesByWorktree', 'expandedDirs', - 'lastVisitedAtByWorktreeId', 'defaultTerminalTabsAppliedByWorktreeId', 'recentlyClosedTabKindsByWorktree' ] as const satisfies readonly (keyof AppState)[] @@ -98,6 +98,24 @@ export function buildWorktreeRenameState( for (const key of WORKTREE_ID_KEYED_MAP_KEYS) { renameKey(key, renameValueByKey[key] as ((value: unknown) => unknown) | undefined) } + // Recency keys may carry a host prefix. Preserve that prefix while moving + // the path-derived id so a rename cannot merge host twins. + const nextVisitRecency = { ...s.lastVisitedAtByWorktreeId } + let visitRecencyChanged = false + for (const [key, value] of Object.entries(s.lastVisitedAtByWorktreeId)) { + const rawId = getWorktreeIdFromVisitKey(key) + if (rawId !== oldWorktreeId) { + continue + } + const nextKey = + rawId === key ? newWorktreeId : `${key.slice(0, key.length - rawId.length)}${newWorktreeId}` + nextVisitRecency[nextKey] = value + delete nextVisitRecency[key] + visitRecencyChanged = true + } + if (visitRecencyChanged) { + renamed.lastVisitedAtByWorktreeId = nextVisitRecency + } // Re-key on rename so a renamed worktree keeps its editor-undo + push/pull state. renameKey('recentlyClosedEditorTabsByWorktree', (files: { worktreeId: string }[]) => files.map(withNewWorktreeId) diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts b/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts index f63712427c5..c6fd3143b17 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-slice-lookups.ts @@ -82,11 +82,14 @@ export function createPurgeWorktreeTerminalState( set: WorktreeSliceSet, _get: WorktreeSliceGet ): WorktreeSlice['purgeWorktreeTerminalState'] { - return (worktreeIds: string[]) => { - const purgeableWorktreeIds = worktreeIds.filter((id) => id !== FLOATING_TERMINAL_WORKTREE_ID) - if (purgeableWorktreeIds.length === 0) { + return (worktreeTargets) => { + const purgeableWorktreeTargets = worktreeTargets.filter((target) => { + const worktreeId = typeof target === 'string' ? target : target.id + return worktreeId !== FLOATING_TERMINAL_WORKTREE_ID + }) + if (purgeableWorktreeTargets.length === 0) { return } - set((s) => buildWorktreePurgeState(s, purgeableWorktreeIds)) + set((s) => buildWorktreePurgeState(s, purgeableWorktreeTargets)) } } diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts b/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts index 76776597d7e..5cfc5eef3d2 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-visit-recency.ts @@ -2,24 +2,39 @@ import type { WorktreeSlice } from '../../worktree-helpers' import type { WorktreeSliceGet, WorktreeSliceSet } from '../listing/worktree-slice-types' import { getRepoIdFromWorktreeId } from '../../worktree-helpers' import { worktreeWorkspaceKey } from '../../../../../../shared/workspace-scope' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' +import { + getWorktreeIdFromVisitKey, + getWorktreeVisitKey, + getWorktreeVisitTimestamp +} from '@/lib/worktree-visit-recency' export function createMarkWorktreeVisited( set: WorktreeSliceSet, _get: WorktreeSliceGet ): WorktreeSlice['markWorktreeVisited'] { - return (worktreeId, visitedAt) => { + return (worktreeId, visitedAt, executionHostId) => { // Why: Cmd+J empty-query ordering needs a focus-recency signal distinct from lastActivityAt (background PTY/activity). // Monotonic: CLI/IPC activations can race, so older timestamps must not regress. See docs/cmd-j-empty-query-ordering.md. set((s) => { const now = visitedAt ?? Date.now() - const prev = s.lastVisitedAtByWorktreeId[worktreeId] ?? 0 + const ownerHostId = + executionHostId ?? + (s.activeWorktreeId === worktreeId ? s.activeWorkspaceExecutionHostId : undefined) ?? + s.getKnownWorktreeById(worktreeId, executionHostId)?.hostId + const visitKey = getWorktreeVisitKey(worktreeId, ownerHostId) + const prev = + getWorktreeVisitTimestamp(s.lastVisitedAtByWorktreeId, { + id: worktreeId, + hostId: ownerHostId + }) ?? 0 if (!(now > prev)) { return {} } return { lastVisitedAtByWorktreeId: { ...s.lastVisitedAtByWorktreeId, - [worktreeId]: now + [visitKey]: now } } }) @@ -35,30 +50,41 @@ export function createPruneLastVisitedTimestamps( // Why: prune per-repo, not globally — SSH repos aren't hydrated at startup, so a global prune would wipe SSH focus-recency. // Only drop for repos with a populated/authoritative list; a missing repoId means not-yet-hydrated (defer). const validIdsByRepo = new Map>() + const validWorktreeIds = new Set() + const validVisitKeys = new Set() + const addValidWorktree = (worktree: { id: string; hostId?: ExecutionHostId }): void => { + validWorktreeIds.add(worktree.id) + validVisitKeys.add(getWorktreeVisitKey(worktree.id, worktree.hostId)) + } for (const [repoId, list] of Object.entries(s.worktreesByRepo)) { // An empty list is the not-yet-hydrated shape, not an authoritative "no worktrees". if (s.detectedWorktreesByRepo[repoId] || list.length === 0) { continue } validIdsByRepo.set(repoId, new Set(list.map((worktree) => worktree.id))) + list.forEach(addValidWorktree) } for (const [repoId, result] of Object.entries(s.detectedWorktreesByRepo)) { if (result.authoritative) { validIdsByRepo.set(repoId, new Set(result.worktrees.map((worktree) => worktree.id))) + result.worktrees.forEach(addValidWorktree) } } let changed = false const next: Record = {} - for (const [id, ts] of Object.entries(s.lastVisitedAtByWorktreeId)) { + for (const [key, ts] of Object.entries(s.lastVisitedAtByWorktreeId)) { + const id = getWorktreeIdFromVisitKey(key) const repoId = getRepoIdFromWorktreeId(id) const repoIds = validIdsByRepo.get(repoId) if (!repoIds) { // Repo not yet hydrated (e.g. SSH not connected). Keep the entry. - next[id] = ts + next[key] = ts continue } - if (repoIds.has(id)) { - next[id] = ts + // Legacy bare keys remain valid when any host still publishes the id; + // qualified keys must match the exact host-qualified row. + if (getWorktreeIdFromVisitKey(key) === key ? repoIds.has(id) : validVisitKeys.has(key)) { + next[key] = ts } else { changed = true } @@ -113,13 +139,15 @@ export function createSeedActiveWorktreeLastVisitedIfMissing( if (!id) { return {} } - if (s.lastVisitedAtByWorktreeId[id] != null) { + const hostId = s.activeWorkspaceExecutionHostId ?? s.getKnownWorktreeById(id)?.hostId + const key = getWorktreeVisitKey(id, hostId) + if (getWorktreeVisitTimestamp(s.lastVisitedAtByWorktreeId, { id, hostId }) != null) { return {} } return { lastVisitedAtByWorktreeId: { ...s.lastVisitedAtByWorktreeId, - [id]: Date.now() + [key]: Date.now() } } }) diff --git a/src/renderer/src/store/slices/worktrees/teardown/purge-stale-runtime-host-state.ts b/src/renderer/src/store/slices/worktrees/teardown/purge-stale-runtime-host-state.ts index a23f0707d91..8894499b714 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/purge-stale-runtime-host-state.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/purge-stale-runtime-host-state.ts @@ -1,4 +1,4 @@ -import type { WorktreeSlice } from '../../worktree-helpers' +import type { WorktreePurgeTarget, WorktreeSlice } from '../../worktree-helpers' import type { WorktreeSliceGet, WorktreeSliceSet } from '../listing/worktree-slice-types' import type { ProjectHostSetup } from '../../../../../../shared/project-types' import type { Repo } from '../../../../../../shared/repo-types' @@ -15,6 +15,7 @@ import { isRemovedRuntimeHostId } from '../../stale-runtime-host-rows' import { buildWorktreePurgeState } from './worktree-purge-state' +import { removeWorktreeVisitEntriesForTargets } from '@/lib/worktree-visit-recency' export function createPurgeStaleRuntimeHostState( set: WorktreeSliceSet, @@ -86,6 +87,8 @@ export function createPurgeStaleRuntimeHostState( recordWorktreeOwners(s.worktreesByRepo) recordWorktreeOwners(detectedRows) + const removedWorktreeTargets: WorktreePurgeTarget[] = [] + const seenRemovedWorktreeTargets = new Set() const sessionWorktreeIdsOwnedByRemovedHosts = new Set() let survivingRestoredSessionOwners = s.restoredRuntimeHostIdByWorkspaceSessionKey for (const [workspaceKey, hostId] of Object.entries( @@ -103,6 +106,12 @@ export function createPurgeStaleRuntimeHostState( continue } sessionWorktreeIdsOwnedByRemovedHosts.add(worktreeId) + // Restored session ownership is authoritative even before catalog rows hydrate. + const identity = `${hostId}\u0000${worktreeId}` + if (!seenRemovedWorktreeTargets.has(identity)) { + seenRemovedWorktreeTargets.add(identity) + removedWorktreeTargets.push({ id: worktreeId, hostId }) + } repoIdsWithRemovedOwners.add(repoId) if (survivingRestoredSessionOwners === s.restoredRuntimeHostIdByWorkspaceSessionKey) { survivingRestoredSessionOwners = { ...survivingRestoredSessionOwners } @@ -116,6 +125,55 @@ export function createPurgeStaleRuntimeHostState( repoIdsWithoutSurvivingOwners.delete(repoId) } + // Preserve host ownership for recency teardown. The other bulk maps are + // keyed by raw worktree id, but a host-qualified visit must not be + // removed when an id twin survives on another host. + const addRemovedWorktreeTarget = ( + worktreeId: string, + hostId: ExecutionHostId | undefined + ): void => { + const identity = `${hostId ?? ''}\u0000${worktreeId}` + if (seenRemovedWorktreeTargets.has(identity)) { + return + } + seenRemovedWorktreeTargets.add(identity) + removedWorktreeTargets.push(hostId ? { id: worktreeId, hostId } : { id: worktreeId }) + } + const recordRemovedRowTargets = ( + rowsByRepo: Record< + string, + readonly { + id: string + hostId?: ExecutionHostId + runtimeOwnerEnvironmentId?: string + }[] + > + ): void => { + for (const [repoId, rows] of Object.entries(rowsByRepo)) { + for (const row of rows) { + const removedOwner = + (row.runtimeOwnerEnvironmentId !== undefined && + removed.has(row.runtimeOwnerEnvironmentId)) || + isRemovedRuntimeHostId(row.hostId, removed) || + (row.hostId === undefined && repoIdsWithoutSurvivingOwners.has(repoId)) + if (!removedOwner) { + continue + } + // A row's execution host is the recency owner. Runtime ownership + // is the only host evidence available for legacy rows without it. + addRemovedWorktreeTarget( + row.id, + row.hostId ?? + (row.runtimeOwnerEnvironmentId + ? toRuntimeExecutionHostId(row.runtimeOwnerEnvironmentId) + : undefined) + ) + } + } + } + recordRemovedRowTargets(s.worktreesByRepo) + recordRemovedRowTargets(detectedRows) + const worktreeDrop = dropWorktreeRowsForRemovedRuntimeEnvironments( s.worktreesByRepo, removed, @@ -163,8 +221,24 @@ export function createPurgeStaleRuntimeHostState( } } } - const purgeState = - removedWorktreeIds.size > 0 ? buildWorktreePurgeState(s, [...removedWorktreeIds]) : {} + const purgeTargets = [...removedWorktreeIds].flatMap((worktreeId) => { + const targets = removedWorktreeTargets.filter((target) => target.id === worktreeId) + return targets.length > 0 ? targets : [{ id: worktreeId }] + }) + const purgeState = purgeTargets.length > 0 ? buildWorktreePurgeState(s, purgeTargets) : {} + const visitPurgeTargets = [ + ...removedWorktreeTargets, + ...purgeTargets.filter( + (target) => !removedWorktreeTargets.some((candidate) => candidate.id === target.id) + ) + ] + const nextLastVisitedAtByWorktreeId = removeWorktreeVisitEntriesForTargets( + s.lastVisitedAtByWorktreeId, + visitPurgeTargets + ) + if (nextLastVisitedAtByWorktreeId !== s.lastVisitedAtByWorktreeId) { + purgeState.lastVisitedAtByWorktreeId = nextLastVisitedAtByWorktreeId + } const restoredSessionOwnersChanged = survivingRestoredSessionOwners !== s.restoredRuntimeHostIdByWorkspaceSessionKey diff --git a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts index 37486927717..08613ebdc45 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/remove-worktree-store-cleanup.ts @@ -1,11 +1,14 @@ import { worktreeWorkspaceKey } from '../../../../../../shared/workspace-scope' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' import type { WorktreeSliceSet } from '../listing/worktree-slice-types' import { removeDeleteStatesForWorktreeIds } from './worktree-delete-state' +import { removeWorktreeVisitEntries } from '@/lib/worktree-visit-recency' export function applyRemoveWorktreeSuccessState( set: WorktreeSliceSet, worktreeId: string, - tabIds: Set + tabIds: Set, + executionHostId?: ExecutionHostId ): void { set((s) => { const next = { ...s.worktreesByRepo } @@ -147,14 +150,11 @@ export function applyRemoveWorktreeSuccessState( const nextEverActivatedWorktreeIds = s.everActivatedWorktreeIds.has(worktreeId) ? new Set([...s.everActivatedWorktreeIds].filter((id) => id !== worktreeId)) : s.everActivatedWorktreeIds - const nextLastVisitedAtByWorktreeId = - worktreeId in s.lastVisitedAtByWorktreeId - ? (() => { - const next = { ...s.lastVisitedAtByWorktreeId } - delete next[worktreeId] - return next - })() - : s.lastVisitedAtByWorktreeId + const nextLastVisitedAtByWorktreeId = removeWorktreeVisitEntries( + s.lastVisitedAtByWorktreeId, + new Set([worktreeId]), + executionHostId + ) return { worktreesByRepo: next, worktreeLineageById: nextLineage, diff --git a/src/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.ts b/src/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.ts index 971d8f06397..329266ae369 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/removed-worktree-renderer-teardown.ts @@ -59,7 +59,7 @@ export async function tearDownRemovedWorktreeRendererState(args: { // Why: dispose parked terminal watchers only on explicit deletion; identity migration/remounts must keep buffered PTY state. disposeRemovedWorktreeParkedTerminalWatchers(worktreeId, terminalPtyIdsBeforeRemoval) - applyRemoveWorktreeSuccessState(set, worktreeId, tabIds) + applyRemoveWorktreeSuccessState(set, worktreeId, tabIds, requiredExecutionHostId ?? hostId) get().removeWorkspaceSpaceWorktrees?.( hostId ? [{ id: worktreeId, executionHostId: hostId }] : [worktreeId] ) diff --git a/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts b/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts index 4445dd306bc..68b5b2883c3 100644 --- a/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts +++ b/src/renderer/src/store/slices/worktrees/teardown/worktree-purge-state.ts @@ -1,13 +1,21 @@ import type { AppState } from '../../../types' +import type { WorktreePurgeTarget, WorktreePurgeTargets } from '../../worktree-helpers' import { forgetHugeRepoWarningDismissalsForWorktrees } from '@/lib/source-control-huge-repo-warning-dismissals' import { parseWorkspaceKey } from '../../../../../../shared/workspace-scope' import { pruneHostedReviewLinkMutationGenerations } from '../metadata/hosted-review-link-mutation' import { collectWorktreePurgeDoomedIds } from './worktree-purge-doomed-ids' import { createWorktreePurgeOmitters } from './worktree-purge-omitters' import { removeDeleteStatesForWorktreeIds } from './worktree-delete-state' +import { removeWorktreeVisitEntriesForTargets } from '@/lib/worktree-visit-recency' -export function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Partial { - const worktreeIdSet = new Set(worktreeIds) +export function buildWorktreePurgeState( + s: AppState, + worktreeTargets: WorktreePurgeTargets +): Partial { + const normalizedTargets: WorktreePurgeTarget[] = worktreeTargets.map((target) => + typeof target === 'string' ? { id: target } : target + ) + const worktreeIdSet = new Set(normalizedTargets.map((target) => target.id)) pruneHostedReviewLinkMutationGenerations(worktreeIdSet) // Why: every authoritative and explicit purge converges here, so a deleted path can't inherit stale UI state. forgetHugeRepoWarningDismissalsForWorktrees(worktreeIdSet) @@ -172,7 +180,10 @@ export function buildWorktreePurgeState(s: AppState, worktreeIds: string[]): Par // Top-level actives openFiles: nextOpenFiles, everActivatedWorktreeIds: nextEverActivatedWorktreeIds, - lastVisitedAtByWorktreeId: omitByWorktree(s.lastVisitedAtByWorktreeId), + lastVisitedAtByWorktreeId: removeWorktreeVisitEntriesForTargets( + s.lastVisitedAtByWorktreeId, + normalizedTargets + ), // Why: keyed by worktreeId; re-keyed on rename but missed by both removal paths (write-once default-terminal guard). defaultTerminalTabsAppliedByWorktreeId: omitByWorktree( s.defaultTerminalTabsAppliedByWorktreeId diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index f6b5fbd473a..5e492168b58 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -945,6 +945,7 @@ function createWebPreloadApi(): Partial { onLegacyWorkerTerminalRecovery: () => noopUnsubscribe, getMigrationUnsupportedSnapshot: () => Promise.resolve([]), drop: () => {}, + reconcileEndedProcess: () => {}, dropByTabPrefix: () => {}, retirePaneAuthority: () => {}, restorePaneAuthority: () => {}, diff --git a/src/shared/agent-hook-listener-session-replacement.test.ts b/src/shared/agent-hook-listener-session-replacement.test.ts new file mode 100644 index 00000000000..918840aac57 --- /dev/null +++ b/src/shared/agent-hook-listener-session-replacement.test.ts @@ -0,0 +1,209 @@ +import { describe, expect, it } from 'vitest' +import { + createHookListenerState, + normalizeHookPayload, + type HookListenerState +} from './agent-hook-listener' +import { makePaneKey } from './stable-pane-id' + +const LEAF_ID = '44444444-4444-4444-8444-444444444444' +const SESSION_A = 'session-a' +const SESSION_B = 'session-b' + +function claudeEvent( + state: HookListenerState, + paneKey: string, + payload: Record +): ReturnType { + return normalizeHookPayload(state, 'claude', { paneKey, payload }, 'production') +} + +/** Close a turn for `sessionId`, carrying whatever inventory the caller wants folded. */ +function stop( + state: HookListenerState, + paneKey: string, + sessionId: string, + extra: Record = {} +): ReturnType { + return claudeEvent(state, paneKey, { + hook_event_name: 'Stop', + session_id: sessionId, + ...extra + }) +} + +// A replacement session is the only evidence Orca gets for the exits Claude emits no terminating +// hook for (/clear, relaunch, resume). See PLAN-STA-4612 §4.1. +describe('Claude session replacement voids the replaced session claims', () => { + it('voids a session cron gate held by the previous session', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('cron-gate', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + + const replaced = stop(state, paneKey, SESSION_B) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(false) + expect(replaced?.payload.state).toBe('done') + }) + + it('voids a one-shot subagent the previous session was still running', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('roster-void', LEAF_ID) + + claudeEvent(state, paneKey, { + hook_event_name: 'UserPromptSubmit', + session_id: SESSION_A, + prompt: 'go' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'SubagentStart', + session_id: SESSION_A, + agent_id: 'achild-0000000000000001' + }) + expect(state.claudeSubagentRosterByPaneKey.get(paneKey)?.size).toBe(1) + + const replaced = stop(state, paneKey, SESSION_B) + + expect(state.claudeSubagentRosterByPaneKey.has(paneKey)).toBe(false) + expect(replaced?.payload.state).toBe('done') + expect(replaced?.payload.subagents ?? []).toEqual([]) + }) + + it('keeps a confirmed teammate across the replacement — a lead swap cannot end an in-process teammate', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('teammate-kept', LEAF_ID) + + // Anchor the owning session on a LEAD event first. Subagent/teammate events branch out of + // normalizeClaudeEvent before the void ever runs, so without this the replacement below finds + // no previous owner, returns early, and the assertion certifies a guard it never exercised. + claudeEvent(state, paneKey, { + hook_event_name: 'UserPromptSubmit', + session_id: SESSION_A, + prompt: 'spin up the team' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'SubagentStart', + session_id: SESSION_A, + agent_id: 'arev-0000000000000001', + agent_type: 'rev' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'TeammateIdle', + session_id: SESSION_A, + teammate_name: 'rev' + }) + claudeEvent(state, paneKey, { + hook_event_name: 'SubagentStart', + session_id: SESSION_A, + agent_id: 'arev-0000000000000001', + agent_type: 'rev' + }) + const tracked = state.claudeSubagentRosterByPaneKey.get(paneKey)?.get('arev-0000000000000001') + expect(tracked?.confirmedTeammate).toBe(true) + expect(tracked?.state).toBe('working') + expect(state.claudeSessionOwnerByPaneKey.get(paneKey)).toBe(SESSION_A) + + stop(state, paneKey, SESSION_B) + + // The replacement really happened, so the surviving row below is the guard's doing. + expect(state.claudeSessionOwnerByPaneKey.get(paneKey)).toBe(SESSION_B) + + expect( + state.claudeSubagentRosterByPaneKey.get(paneKey)?.get('arev-0000000000000001')?.state + ).toBe('working') + }) + + it('KEEPS a running background-shell gate across the replacement', () => { + // /clear keeps the Claude process alive, so a background shell can outlive the conversation and + // the previous inventory is positive evidence it was running. Retiring it here would decide the + // open live-shell product question inside this change. See PLAN-STA-4612 §6.3. + const state = createHookListenerState() + const paneKey = makePaneKey('bg-gate-kept', LEAF_ID) + + stop(state, paneKey, SESSION_A, { + background_tasks: [{ type: 'bash', status: 'running', id: 'bash_1' }] + }) + expect(state.claudeRunningNonAgentTaskPaneKeys.has(paneKey)).toBe(true) + + const replaced = claudeEvent(state, paneKey, { + hook_event_name: 'PostToolUse', + session_id: SESSION_B + }) + + expect(state.claudeRunningNonAgentTaskPaneKeys.has(paneKey)).toBe(true) + expect(replaced?.payload.state).toBe('working') + }) + + it('voids nothing when the session id is unchanged', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('same-session', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + const same = stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + expect(same?.payload.state).toBe('working') + }) + + it('voids nothing when a CHILD event carries a foreign session id', () => { + // Claude has no equivalent of the Codex child-session guard, so a child payload's own + // session_id reaches the same extraction path as the lead's. + const state = createHookListenerState() + const paneKey = makePaneKey('child-session', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + claudeEvent(state, paneKey, { + hook_event_name: 'PostToolUse', + session_id: 'child-session', + agent_id: 'achild-0000000000000009' + }) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + expect(state.claudeSessionOwnerByPaneKey.get(paneKey)).toBe(SESSION_A) + }) + + it('voids nothing when the event carries no session id', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('no-session', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + stop(state, paneKey, '') + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(true) + expect(state.claudeSessionOwnerByPaneKey.get(paneKey)).toBe(SESSION_A) + }) + + it('does not anchor an ignored compact SessionStart', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('compact-owner', LEAF_ID) + + stop(state, paneKey, SESSION_A, { session_crons: [{ id: 'cron-1' }] }) + expect( + claudeEvent(state, paneKey, { + hook_event_name: 'SessionStart', + session_id: SESSION_B, + source: 'compact' + }) + ).toBeNull() + + stop(state, paneKey, SESSION_B) + + expect(state.claudeActiveSessionCronPaneKeys.has(paneKey)).toBe(false) + }) + + it('leaves the lead-state record to the incoming fold rather than deleting it', () => { + const state = createHookListenerState() + const paneKey = makePaneKey('lead-untouched', LEAF_ID) + + stop(state, paneKey, SESSION_A) + claudeEvent(state, paneKey, { + hook_event_name: 'UserPromptSubmit', + session_id: SESSION_B, + prompt: 'new conversation' + }) + + expect(state.claudeLeadStateByPaneKey.get(paneKey)?.state).toBe('working') + }) +}) diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index ccfabb603df..ecc0d986367 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -142,6 +142,10 @@ export type HookListenerState = { claudeRunningNonAgentTaskPaneKeys: Set /** Panes whose latest authoritative Claude cron inventory still has a scheduled job. */ claudeActiveSessionCronPaneKeys: Set + /** Claude `session_id` that last reported on the pane from a LEAD event. A different id means the + * conversation was replaced (/clear, relaunch, resume), so claims the old session owned are void + * even when no SessionStart arrives — the backstop for the exits that emit no terminating hook. */ + claudeSessionOwnerByPaneKey: Map /** Live thread-spawn children per Codex pane. */ codexSubagentRosterByPaneKey: Map /** Incremental parent/child rollout cursors for Codex collaboration v2. */ @@ -182,6 +186,7 @@ export function createHookListenerState(): HookListenerState { claudeUnconfirmedRestoredStatusPaneKeys: new Set(), claudeRunningNonAgentTaskPaneKeys: new Set(), claudeActiveSessionCronPaneKeys: new Set(), + claudeSessionOwnerByPaneKey: new Map(), codexSubagentRosterByPaneKey: new Map(), codexSubagentTranscriptByPaneKey: new Map(), codexLeadStateByPaneKey: new Map() @@ -199,11 +204,32 @@ export function clearPaneCacheState(state: HookListenerState, paneKey: string): state.claudeUnconfirmedRestoredStatusPaneKeys.delete(paneKey) state.claudeRunningNonAgentTaskPaneKeys.delete(paneKey) state.claudeActiveSessionCronPaneKeys.delete(paneKey) + state.claudeSessionOwnerByPaneKey.delete(paneKey) state.codexSubagentRosterByPaneKey.delete(paneKey) state.codexSubagentTranscriptByPaneKey.delete(paneKey) state.codexLeadStateByPaneKey.delete(paneKey) } +/** Does this pane still hold anything that can ASSERT a state — a stored row, or a Claude latch that + * `resolveClaudePaneState` would re-gate `working` from on the pane's next event? + * + * Deliberately lives next to `clearPaneCacheState` above and enumerates the claim-bearing subset of + * what that function deletes: the two must be edited together, and keeping them three lines apart in + * one file is what makes that obvious. Prompt/tool/transcript caches are excluded — they render a + * row, they never create one. */ +export function paneHasStateClaims(state: HookListenerState, paneKey: string): boolean { + return ( + state.lastStatusByPaneKey.has(paneKey) || + state.claudeSubagentRosterByPaneKey.has(paneKey) || + state.claudeLeadStateByPaneKey.has(paneKey) || + state.claudeRunningNonAgentTaskPaneKeys.has(paneKey) || + state.claudeActiveSessionCronPaneKeys.has(paneKey) || + state.claudeSessionOwnerByPaneKey.has(paneKey) || + state.codexSubagentRosterByPaneKey.has(paneKey) || + state.codexLeadStateByPaneKey.has(paneKey) + ) +} + function movePaneScopedMapEntries( map: Map, fromPaneKey: string, @@ -246,6 +272,7 @@ export function movePaneCacheState( movePaneScopedSetEntries(state.claudeUnconfirmedRestoredStatusPaneKeys, fromPaneKey, toPaneKey) movePaneScopedSetEntries(state.claudeRunningNonAgentTaskPaneKeys, fromPaneKey, toPaneKey) movePaneScopedSetEntries(state.claudeActiveSessionCronPaneKeys, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.claudeSessionOwnerByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.codexSubagentRosterByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.codexSubagentTranscriptByPaneKey, fromPaneKey, toPaneKey) movePaneScopedMapEntries(state.codexLeadStateByPaneKey, fromPaneKey, toPaneKey) @@ -291,6 +318,7 @@ export function clearAllListenerCaches(state: HookListenerState): void { state.claudeUnconfirmedRestoredStatusPaneKeys.clear() state.claudeRunningNonAgentTaskPaneKeys.clear() state.claudeActiveSessionCronPaneKeys.clear() + state.claudeSessionOwnerByPaneKey.clear() state.codexSubagentRosterByPaneKey.clear() state.codexSubagentTranscriptByPaneKey.clear() state.codexLeadStateByPaneKey.clear() @@ -2631,6 +2659,9 @@ function updateClaudeRunningNonAgentTask( state: HookListenerState, paneKey: string, hasRunningNonAgentTask: boolean, + /** Lead-turn property. Pass `false` from any non-lead fold: an interrupt clears the gate even when + * the inventory positively reports a running shell, which is a live-shell judgement no new call + * site may inherit by copying this signature. */ interrupted: boolean ): void { if (hasRunningNonAgentTask && !interrupted) { @@ -2914,6 +2945,84 @@ function buildClaudeCachedLeadStatusPayload( }) } +/** Lead events that may re-anchor a pane's owning session. Allow-list, not a deny-list: a payload we + * can't attribute (unknown name, child event missing its agent_id) must void nothing. */ +const CLAUDE_SESSION_OWNER_EVENTS: ReadonlySet = new Set([ + 'SessionStart', + 'UserPromptSubmit', + 'Stop', + 'StopFailure', + 'PreToolUse', + 'PostToolUse', + 'PostToolUseFailure', + 'PermissionRequest' +]) + +/** A pane whose `session_id` changed is running a different conversation, so claims the previous one + * owned are void — the hook-independent backstop for /clear, relaunch and resume, which emit no + * terminating hook (SessionEnd covers about a third of exit paths). + * + * Voids only what the replaced session provably owned. Deliberately NOT voided: + * - `claudeRunningNonAgentTaskPaneKeys`: a background shell is an OS process that survives /clear, + * and the previous inventory is positive evidence it was running. Only a fresh inventory or a + * certified process death may retire it. + * - `confirmedTeammate` roster rows: persistent in-process teammates a lead replacement can't end. + * - `claudeLeadStateByPaneKey`: the caller's own fold overwrites it anyway. */ +function voidClaimsOfReplacedClaudeSession( + state: HookListenerState, + eventName: unknown, + eventAgentId: string | undefined, + paneKey: string, + hookPayload: Record +): void { + if ( + eventAgentId !== undefined || + typeof eventName !== 'string' || + !CLAUDE_SESSION_OWNER_EVENTS.has(eventName) + ) { + return + } + // Compact/unknown SessionStart events are intentionally ignored by the status fold; do not let + // them advance the owner anchor or the next real lead event will miss the replacement. + if ( + eventName === 'SessionStart' && + hookPayload['source'] !== 'startup' && + hookPayload['source'] !== 'resume' && + hookPayload['source'] !== 'clear' + ) { + return + } + const sessionId = readString(hookPayload, 'session_id') + if (!sessionId) { + return + } + const previousOwner = state.claudeSessionOwnerByPaneKey.get(paneKey) + state.claudeSessionOwnerByPaneKey.set(paneKey, sessionId) + if (previousOwner === undefined || previousOwner === sessionId) { + return + } + // Why: a compact restart mints a SessionStart mid-turn under the same conversation; the existing + // handler already fails closed on non-idle sources, and this must not undercut it. No other + // allow-listed event carries a compact `trigger`, so SessionStart is the whole guard — if a + // compact event is ever added to CLAUDE_SESSION_OWNER_EVENTS, re-derive one for it deliberately. + if (eventName === 'SessionStart') { + return + } + state.claudeActiveSessionCronPaneKeys.delete(paneKey) + const roster = state.claudeSubagentRosterByPaneKey.get(paneKey) + if (!roster) { + return + } + for (const [id, tracked] of roster) { + if (tracked.confirmedTeammate !== true) { + roster.delete(id) + } + } + if (roster.size === 0) { + state.claudeSubagentRosterByPaneKey.delete(paneKey) + } +} + function normalizeClaudeEvent( state: HookListenerState, eventName: unknown, @@ -2929,6 +3038,7 @@ function normalizeClaudeEvent( ) { return normalizeClaudeSubagentLifecycleEvent(state, eventName, paneKey, hookPayload) } + voidClaimsOfReplacedClaudeSession(state, eventName, eventAgentId, paneKey, hookPayload) if (eventName === 'SessionStart') { // Why: SessionStart is the only signal a resumed session emits before its first prompt // (STA-3386). Land it as a session-boundary 'done' row: 'working' would show a phantom diff --git a/src/shared/agent-status-observation.ts b/src/shared/agent-status-observation.ts index 0eeb60e4bb3..d0712d217c5 100644 --- a/src/shared/agent-status-observation.ts +++ b/src/shared/agent-status-observation.ts @@ -62,6 +62,17 @@ export type AgentStatusObservation = { * body, so a hook or OSC writer cannot declare its own provenance. */ export type WithAgentStatusObservation = { observation?: AgentStatusObservation } +/** Renderer-local count of ACCEPTED status writes this pane's row has taken. Incremented only by + * the store's accept branch, off the row it replaces, and carried through by every field-level + * rewrite — so it answers "did the pane report again?", which `updatedAt` cannot, because the + * accept rule admits equal timestamps. Lives on the row rather than in a side table so no + * teardown path can reset it out from under a reader (STA-4612). Never sent over IPC or + * persisted to last-status.json. + * + * Declared here beside the observation facet because both are per-write facets mixed into + * `AgentStatusEntry` rather than fields a reporter supplies. */ +export type AgentStatusRowFacets = WithAgentStatusObservation & { acceptedStatusSeq?: number } + // ─── THE ORDERING RULE ────────────────────────────────────────────────────── // `(authorityId, incarnation, revision)` is a total order ONLY within one authorityId. // A different authorityId means "incomparable", not "older" — the id is regenerated per diff --git a/src/shared/agent-status-types.ts b/src/shared/agent-status-types.ts index 6df1c6568f7..714b47d4a80 100644 --- a/src/shared/agent-status-types.ts +++ b/src/shared/agent-status-types.ts @@ -3,7 +3,7 @@ // a narrow interrupt fallback synthesizes a final `done` when an agent misses its cancellation hook. import type { AgentProviderSessionMetadata } from './agent-session-resume' -import type { WithAgentStatusObservation } from './agent-status-observation' +import type { AgentStatusRowFacets, WithAgentStatusObservation } from './agent-status-observation' import { normalizeInteractivePromptField, normalizeOptionalField, @@ -152,7 +152,7 @@ export type AgentStatusEntry = { * the transition may have been missed while no receiver was up, so freshness gates * treat the row as stale immediately. Cleared by any accepted live event. */ restoredUnconfirmed?: boolean -} & WithAgentStatusObservation +} & AgentStatusRowFacets export type MigrationUnsupportedPtyEntry = { ptyId: string diff --git a/src/shared/remote-workspace-session-projection.ts b/src/shared/remote-workspace-session-projection.ts index 3b60d9044ff..32ac8759527 100644 --- a/src/shared/remote-workspace-session-projection.ts +++ b/src/shared/remote-workspace-session-projection.ts @@ -3,13 +3,21 @@ import type { RemoteWorkspaceSession, RemoteWorkspaceTerminalTab } from './remot import type { TerminalTab } from './terminal-tab-types' import type { WorkspaceSessionState } from './workspace-session-state-types' import { splitWorktreeId } from './worktree/id' +import type { ExecutionHostId } from './execution-host' +import { + composeWorktreeHostIdentity, + getWorktreeIdFromHostIdentity, + isWorktreeHostIdentity +} from './worktree/host-qualified-identity' type ExportOptions = { - isTargetWorktree: (worktreeId: string) => boolean + isTargetWorktree: (worktreeId: string, executionHostId?: ExecutionHostId) => boolean } type ImportOptions = { resolveWorktreeId: (worktreePath: string) => string | null + /** Host owning the projected snapshot; absent preserves the legacy bare key. */ + executionHostId?: ExecutionHostId } function worktreePathFromId(worktreeId: string): string | null { @@ -70,13 +78,23 @@ export function exportRemoteWorkspaceSession( } const lastVisitedAtByWorktreePath: Record = {} - for (const [worktreeId, timestamp] of Object.entries(session.lastVisitedAtByWorktreeId ?? {})) { - if (!options.isTargetWorktree(worktreeId)) { + for (const [visitKey, timestamp] of Object.entries(session.lastVisitedAtByWorktreeId ?? {})) { + const worktreeId = isWorktreeHostIdentity(visitKey) + ? getWorktreeIdFromHostIdentity(visitKey) + : visitKey + const executionHostId = isWorktreeHostIdentity(visitKey) + ? (visitKey.slice(0, visitKey.indexOf('|')) as ExecutionHostId) + : undefined + if (!options.isTargetWorktree(worktreeId, executionHostId)) { continue } const worktreePath = worktreePathFromId(worktreeId) if (worktreePath) { - lastVisitedAtByWorktreePath[worktreePath] = timestamp + // Why max: a bare legacy key and its host-qualified twin collapse onto one path. + lastVisitedAtByWorktreePath[worktreePath] = Math.max( + lastVisitedAtByWorktreePath[worktreePath] ?? 0, + timestamp + ) } } @@ -101,7 +119,7 @@ export function exportRemoteWorkspaceSession( ) ), activeWorktreePathsOnShutdown: session.activeWorktreeIdsOnShutdown - ?.filter(options.isTargetWorktree) + ?.filter((worktreeId) => options.isTargetWorktree(worktreeId)) .map(worktreePathFromId) .filter((path): path is string => Boolean(path)), activeTabIdByWorktreePath, @@ -166,7 +184,10 @@ export function importRemoteWorkspaceSession( )) { const worktreeId = resolvePath(worktreePath) if (worktreeId) { - lastVisitedAtByWorktreeId[worktreeId] = timestamp + const key = options.executionHostId + ? composeWorktreeHostIdentity(options.executionHostId, worktreeId) + : worktreeId + lastVisitedAtByWorktreeId[key] = timestamp } } diff --git a/src/shared/workspace-cleanup-applied-filters.test.ts b/src/shared/workspace-cleanup-applied-filters.test.ts new file mode 100644 index 00000000000..9cd9fd4afb8 --- /dev/null +++ b/src/shared/workspace-cleanup-applied-filters.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from 'vitest' +import { createDefaultWorkspaceCleanupFilterState } from './workspace-cleanup-filter-model' +import { listAppliedWorkspaceCleanupFilters } from './workspace-cleanup-applied-filters' + +const FORMAT = { + idleDays: (d: number) => `Idle >= ${d}d`, + neverVisited: () => 'Never visited', + minSize: (b: number) => `At least ${b}B`, + maxSize: (b: number) => `At most ${b}B`, + excludesUnsized: () => 'Excludes unmeasured', + excludesStatusless: () => 'Excludes statusless', + list: (kind: string, count: number) => `${kind}:${count}`, + triState: (kind: string, mode: string) => `${kind}:${mode}`, + minAhead: (n: number) => `Ahead >= ${n}`, + minBehind: (n: number) => `Behind >= ${n}`, + branchQuery: (v: string) => `Branch ~ ${v}`, + pathPrefix: (v: string) => `Path ~ ${v}`, + presence: (kind: string, mode: string) => `${kind}:${mode}`, + completelyEmpty: () => 'Nothing to lose', + selectableOnly: () => 'Deletable only' +} + +const list = (f: ReturnType) => + listAppliedWorkspaceCleanupFilters(f, FORMAT) + +describe('listAppliedWorkspaceCleanupFilters', () => { + it('shows nothing for a default profile', () => { + expect(list(createDefaultWorkspaceCleanupFilterState())).toEqual([]) + }) + + it('names the reported stuck filter rather than its group', () => { + // The bar always read "Showing 546 of 799"; what was missing was which filter. + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 20 + + const applied = list(filters) + + expect(applied).toHaveLength(1) + expect(applied[0].label).toBe('Idle >= 20d') + }) + + it('clears one constraint and leaves the rest of its own group alone', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 20 + filters.activity.neverVisited = true + + const idle = list(filters).find((a) => a.id === 'activity.idleMinDays') + const next = idle!.clear(filters) + + expect(next.activity.idleMinDays).toBeNull() + expect(next.activity.neverVisited).toBe(true) + }) + + it('never mutates the filters it is handed', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.location.pathPrefix = '/repos' + + list(filters)[0].clear(filters) + + expect(filters.location.pathPrefix).toBe('/repos') + }) + + describe('numeric zero', () => { + it('hides an inert zero minimum, which matches every row', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 0 + filters.size.minBytes = 0 + filters.git.minAhead = 0 + filters.git.minBehind = 0 + + expect(list(filters)).toEqual([]) + }) + + it('shows a zero maximum, which hides every measured non-empty workspace', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.size.maxBytes = 0 + + expect(list(filters).map((a) => a.id)).toEqual(['size.maxBytes']) + }) + }) + + it('ignores whitespace-only text', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.git.branchQuery = ' ' + filters.location.pathPrefix = ' ' + + expect(list(filters)).toEqual([]) + }) + + it('ignores parameter fields that constrain nothing on their own', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleSignal = 'created' + filters.safety.blockerMode = 'any-of' + + expect(list(filters)).toEqual([]) + }) + + it('shows the permissive booleans only when switched off', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.size.includeUnsized = false + filters.status.matchStatusless = false + + expect(list(filters).map((a) => a.id).sort()).toEqual([ + 'size.includeUnsized', + 'status.matchStatusless' + ]) + }) + + it('gives every constraint a unique, stable id', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 20 + filters.size.maxBytes = 500 + filters.status.archived = 'only' + filters.git.states = ['dirty'] + filters.git.locked = 'exclude' + filters.location.repoIds = ['repo-1'] + filters.safety.dismissed = 'only' + + const ids = list(filters).map((a) => a.id) + + expect(new Set(ids).size).toBe(ids.length) + expect(ids).toContain('status.archived') + expect(ids).toContain('git.locked') + }) + + it('clearing every chip in turn returns the profile to no constraints', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.activity.idleMinDays = 20 + filters.size.maxBytes = 0 + filters.size.includeUnsized = false + filters.status.matchStatusless = false + filters.git.branchQuery = 'release' + filters.location.pathPrefix = '/repos' + filters.safety.selectableOnly = true + + let next = filters + for (const applied of list(filters)) { + next = applied.clear(next) + } + + expect(list(next)).toEqual([]) + }) + + it('leaves the search query alone, which chips do not own', () => { + const filters = createDefaultWorkspaceCleanupFilterState() + filters.query = 'release' + filters.activity.idleMinDays = 20 + + const next = list(filters)[0].clear(filters) + + expect(next.query).toBe('release') + }) +}) diff --git a/src/shared/workspace-cleanup-applied-filters.ts b/src/shared/workspace-cleanup-applied-filters.ts new file mode 100644 index 00000000000..449a55f088f --- /dev/null +++ b/src/shared/workspace-cleanup-applied-filters.ts @@ -0,0 +1,255 @@ +import { + createDefaultWorkspaceCleanupFilterState, + type WorkspaceCleanupFilterState +} from './workspace-cleanup-filter-model' + +/** + * One constraint the user can see and remove from the filter bar. + * + * Deliberately per-field, not per-group: "Activity" tells a reader nothing, while + * "Idle >= 20d" names the thing that is hiding their workspaces. That naming *is* the + * fix — the reported defect was never that the effect was invisible (the bar always + * read "Showing 546 of 799"), it was that the cause was. + */ +export type WorkspaceCleanupAppliedFilter = { + /** Stable across renders and unique per constraint. */ + id: string + /** Rendered on the chip. Already localized by the caller-supplied formatter. */ + label: string + /** Returns `filters` with just this constraint reset to its default. */ + clear: (filters: WorkspaceCleanupFilterState) => WorkspaceCleanupFilterState +} + +type Formatters = { + idleDays: (days: number) => string + neverVisited: () => string + minSize: (bytes: number) => string + maxSize: (bytes: number) => string + excludesUnsized: () => string + excludesStatusless: () => string + list: (kind: string, count: number) => string + triState: (kind: string, mode: 'only' | 'exclude') => string + minAhead: (count: number) => string + minBehind: (count: number) => string + branchQuery: (value: string) => string + pathPrefix: (value: string) => string + presence: (kind: string, mode: string) => string + completelyEmpty: () => string + selectableOnly: () => string +} + +/** + * Every constraint currently narrowing the fleet, in panel order. + * + * A field counts only when its matcher actually narrows. That is why minimums count + * above zero but `size.maxBytes` counts *at* zero: a `0` minimum matches every row, + * while "at most 0 MB" hides every measured non-empty workspace. + */ +export function listAppliedWorkspaceCleanupFilters( + filters: WorkspaceCleanupFilterState, + format: Formatters +): WorkspaceCleanupAppliedFilter[] { + const defaults = createDefaultWorkspaceCleanupFilterState() + const applied: WorkspaceCleanupAppliedFilter[] = [] + const add = ( + id: string, + label: string, + clear: (f: WorkspaceCleanupFilterState) => WorkspaceCleanupFilterState + ): void => { + applied.push({ id, label, clear }) + } + + const { activity, size, status, agent, git, review, ticket, context, location, safety } = filters + + if (activity.idleMinDays !== null && activity.idleMinDays > 0) { + add('activity.idleMinDays', format.idleDays(activity.idleMinDays), (f) => ({ + ...f, + activity: { ...f.activity, idleMinDays: null } + })) + } + if (activity.neverVisited) { + add('activity.neverVisited', format.neverVisited(), (f) => ({ + ...f, + activity: { ...f.activity, neverVisited: false } + })) + } + + if (size.minBytes !== null && size.minBytes > 0) { + add('size.minBytes', format.minSize(size.minBytes), (f) => ({ + ...f, + size: { ...f.size, minBytes: null } + })) + } + if (size.maxBytes !== null) { + add('size.maxBytes', format.maxSize(size.maxBytes), (f) => ({ + ...f, + size: { ...f.size, maxBytes: null } + })) + } + if (!size.includeUnsized) { + add('size.includeUnsized', format.excludesUnsized(), (f) => ({ + ...f, + size: { ...f.size, includeUnsized: true } + })) + } + + if (status.workspaceStatuses.length > 0) { + add('status.workspaceStatuses', format.list('status', status.workspaceStatuses.length), (f) => ({ + ...f, + status: { ...f.status, workspaceStatuses: [] } + })) + } + if (!status.matchStatusless) { + add('status.matchStatusless', format.excludesStatusless(), (f) => ({ + ...f, + status: { ...f.status, matchStatusless: true } + })) + } + for (const key of ['archived', 'pinned', 'unread', 'comment'] as const) { + const value = status[key] + if (value !== 'any') { + add(`status.${key}`, format.triState(key, value), (f) => ({ + ...f, + status: { ...f.status, [key]: 'any' } + })) + } + } + + if (agent.states.length > 0) { + add('agent.states', format.list('agent', agent.states.length), (f) => ({ + ...f, + agent: { ...f.agent, states: [] } + })) + } + if (agent.retainedDoneAgents !== 'any') { + add('agent.retainedDoneAgents', format.triState('retainedAgents', agent.retainedDoneAgents), (f) => ({ + ...f, + agent: { ...f.agent, retainedDoneAgents: 'any' } + })) + } + + if (git.states.length > 0) { + add('git.states', format.list('git', git.states.length), (f) => ({ + ...f, + git: { ...f.git, states: [] } + })) + } + if (git.minAhead !== null && git.minAhead > 0) { + add('git.minAhead', format.minAhead(git.minAhead), (f) => ({ + ...f, + git: { ...f.git, minAhead: null } + })) + } + if (git.minBehind !== null && git.minBehind > 0) { + add('git.minBehind', format.minBehind(git.minBehind), (f) => ({ + ...f, + git: { ...f.git, minBehind: null } + })) + } + if (git.branchQuery.trim().length > 0) { + add('git.branchQuery', format.branchQuery(git.branchQuery.trim()), (f) => ({ + ...f, + git: { ...f.git, branchQuery: '' } + })) + } + for (const key of ['prunable', 'locked'] as const) { + const value = git[key] + if (value !== 'any') { + add(`git.${key}`, format.triState(key, value), (f) => ({ + ...f, + git: { ...f.git, [key]: 'any' } + })) + } + } + + if (review.presence !== 'any') { + add('review.presence', format.presence('review', review.presence), (f) => ({ + ...f, + review: { ...f.review, presence: defaults.review.presence } + })) + } + if (review.states.length > 0) { + add('review.states', format.list('reviewState', review.states.length), (f) => ({ + ...f, + review: { ...f.review, states: [] } + })) + } + if (review.providers.length > 0) { + add('review.providers', format.list('reviewProvider', review.providers.length), (f) => ({ + ...f, + review: { ...f.review, providers: [] } + })) + } + + if (ticket.presence !== 'any') { + add('ticket.presence', format.presence('ticket', ticket.presence), (f) => ({ + ...f, + ticket: { ...f.ticket, presence: defaults.ticket.presence } + })) + } + if (ticket.sources.length > 0) { + add('ticket.sources', format.list('ticketSource', ticket.sources.length), (f) => ({ + ...f, + ticket: { ...f.ticket, sources: [] } + })) + } + + if (context.presence !== 'any') { + add('context.presence', format.presence('context', context.presence), (f) => ({ + ...f, + context: { ...f.context, presence: defaults.context.presence } + })) + } + if (context.completelyEmpty) { + add('context.completelyEmpty', format.completelyEmpty(), (f) => ({ + ...f, + context: { ...f.context, completelyEmpty: false } + })) + } + + if (location.hostIds.length > 0) { + add('location.hostIds', format.list('host', location.hostIds.length), (f) => ({ + ...f, + location: { ...f.location, hostIds: [] } + })) + } + if (location.repoIds.length > 0) { + add('location.repoIds', format.list('repo', location.repoIds.length), (f) => ({ + ...f, + location: { ...f.location, repoIds: [] } + })) + } + if (location.pathPrefix.trim().length > 0) { + add('location.pathPrefix', format.pathPrefix(location.pathPrefix.trim()), (f) => ({ + ...f, + location: { ...f.location, pathPrefix: '' } + })) + } + + if (safety.blockers.length > 0) { + add('safety.blockers', format.list('blocker', safety.blockers.length), (f) => ({ + ...f, + safety: { ...f.safety, blockers: [] } + })) + } + if (safety.tiers.length > 0) { + add('safety.tiers', format.list('tier', safety.tiers.length), (f) => ({ + ...f, + safety: { ...f.safety, tiers: [] } + })) + } + if (safety.dismissed !== 'any') { + add('safety.dismissed', format.triState('dismissed', safety.dismissed), (f) => ({ + ...f, + safety: { ...f.safety, dismissed: 'any' } + })) + } + if (safety.selectableOnly) { + add('safety.selectableOnly', format.selectableOnly(), (f) => ({ + ...f, + safety: { ...f.safety, selectableOnly: false } + })) + } + + return applied +} diff --git a/src/shared/workspace-session-schema.ts b/src/shared/workspace-session-schema.ts index d9aba06ac9d..6420f9e1e60 100644 --- a/src/shared/workspace-session-schema.ts +++ b/src/shared/workspace-session-schema.ts @@ -147,6 +147,8 @@ const tabSchema = z.object({ color: z.string().nullable(), sortOrder: z.number(), createdAt: z.number(), + // Why: corrupt optional recency must not discard the whole persisted tab. + lastFocusedAt: z.number().finite().nonnegative().optional().catch(undefined), isPreview: z.boolean().optional(), isPinned: z.boolean().optional(), // Why: persist the per-tab native-chat view mode so 'chat' survives reload / diff --git a/src/shared/workspace-session-state-types.ts b/src/shared/workspace-session-state-types.ts index bc20b7c5459..4b4c07ba908 100644 --- a/src/shared/workspace-session-state-types.ts +++ b/src/shared/workspace-session-state-types.ts @@ -84,7 +84,9 @@ export type WorkspaceSessionState = { * and worktreeNavHistory (Back/Forward stack). See * docs/cmd-j-empty-query-ordering.md. Absent in sessions written by * older builds — hydration tolerates missing/partial maps and the - * active worktree is seeded on first restore. */ + * active worktree is seeded on first restore. New host-qualified keys use + * `${executionHostId}|${worktreeId}`; legacy bare keys remain readable + * during migration and remote snapshot projection. */ lastVisitedAtByWorktreeId?: Record /** Worktrees whose repo-defined default terminal tabs have already been * considered. Persisted so closing all tabs and re-opening the workspace diff --git a/src/shared/workspace-session-tab-focus-schema.test.ts b/src/shared/workspace-session-tab-focus-schema.test.ts new file mode 100644 index 00000000000..9e2167d5082 --- /dev/null +++ b/src/shared/workspace-session-tab-focus-schema.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vitest' +import { parseWorkspaceSession } from './workspace-session-schema' + +function sessionWithLastFocusedAt(lastFocusedAt: unknown): Record { + return { + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + unifiedTabs: { + wt: [ + { + id: 'tab-1', + entityId: 'terminal-1', + groupId: 'group-1', + worktreeId: 'wt', + contentType: 'terminal', + label: 'Terminal', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1, + lastFocusedAt + } + ] + } + } +} + +describe('workspace session unified-tab focus timestamp', () => { + it('preserves a finite nonnegative lastFocusedAt', () => { + const result = parseWorkspaceSession(sessionWithLastFocusedAt(1_700_000_000_123)) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value.unifiedTabs?.wt[0]?.lastFocusedAt).toBe(1_700_000_000_123) + } + }) + + it.each([-1, Number.NaN, Number.POSITIVE_INFINITY])( + 'drops an invalid lastFocusedAt without dropping its tab (%s)', + (lastFocusedAt) => { + const result = parseWorkspaceSession(sessionWithLastFocusedAt(lastFocusedAt)) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.value.unifiedTabs?.wt).toHaveLength(1) + expect(result.value.unifiedTabs?.wt[0]?.lastFocusedAt).toBeUndefined() + } + } + ) +}) diff --git a/src/shared/worktree/host-qualified-identity.ts b/src/shared/worktree/host-qualified-identity.ts index dcdc4411724..4f5f2fd083e 100644 --- a/src/shared/worktree/host-qualified-identity.ts +++ b/src/shared/worktree/host-qualified-identity.ts @@ -1,5 +1,4 @@ -import type { ExecutionHostId } from '../execution-host' -import { parseExecutionHostId } from '../execution-host' +import { parseExecutionHostId, type ExecutionHostId } from '../execution-host' import type { Worktree } from './types' /** @@ -67,3 +66,12 @@ export function getExecutionHostIdFromWorktreeHostIdentity( export function getWorktreeIdFromHostIdentity(identity: string): string { return identity.slice(identity.indexOf(HOST_SEPARATOR) + 1) } + +/** True only for the canonical host-qualified form, not a legacy id containing `|`. */ +export function isWorktreeHostIdentity(identity: string): boolean { + const separator = identity.indexOf(HOST_SEPARATOR) + return ( + separator === 0 || + (separator > 0 && parseExecutionHostId(identity.slice(0, separator)) !== null) + ) +}