diff --git a/src/main/durable-file-write.ts b/src/main/durable-file-write.ts index 2f40b0881e8..83b0eabc5ed 100644 --- a/src/main/durable-file-write.ts +++ b/src/main/durable-file-write.ts @@ -187,10 +187,15 @@ export async function removeStaleDurableWriteTempFiles( } /** Synchronous counterpart for quit and crash paths that cannot await. */ -export function writeFileDurableSync(tmpPath: string, finalPath: string, payload: string): void { +export function writeFileDurableSync( + tmpPath: string, + finalPath: string, + payload: string | Uint8Array +): void { let renamed = false try { - writeFileSync(tmpPath, payload, 'utf-8') + // A Uint8Array payload is written verbatim; a string still defaults to UTF-8. + writeFileSync(tmpPath, payload) const fd = openSync(tmpPath, 'r+') try { fsyncSync(fd) diff --git a/src/main/persistence/loading-store/normalize-loaded-profile-state.ts b/src/main/persistence/loading-store/normalize-loaded-profile-state.ts index 2286e40d54b..39d625c525a 100644 --- a/src/main/persistence/loading-store/normalize-loaded-profile-state.ts +++ b/src/main/persistence/loading-store/normalize-loaded-profile-state.ts @@ -35,6 +35,8 @@ export function normalizeLoadedProfileState( const { defaults, migratedExternalVisibility, osc52ClipboardNoticePending } = terminal const { normalizedOnboarding, normalizedProjectGroups, loadedCompactWorktreeCards } = profile const projectCatalog = normalizeLoadedProjectCatalog(parsed, markNeedsSave) + // Ordered: the host partitions drop the global fields this slice already owns. + const workspaceSession = normalizeLoadedLocalSession(parsed, defaults, markNeedsSave) return { ...defaults, @@ -69,9 +71,14 @@ export function normalizeLoadedProfileState( markNeedsSave ), // Why: volatile schema; zod-validate workspaceSession at read so a bad payload falls to defaults, not a renderer crash. - workspaceSession: normalizeLoadedLocalSession(parsed, defaults, markNeedsSave), + workspaceSession, // Why: per-host session partitions, validated independently; 'local' stays in workspaceSession for downgrade compat. - workspaceSessionsByHostId: normalizeLoadedHostSessions(parsed, defaults, markNeedsSave), + workspaceSessionsByHostId: normalizeLoadedHostSessions( + parsed, + defaults, + workspaceSession, + markNeedsSave + ), sshTargets: (parsed.sshTargets ?? []).map(normalizeSshTarget), deletedSshConfigAliases: Array.isArray(parsed.deletedSshConfigAliases) ? parsed.deletedSshConfigAliases.filter((alias): alias is string => typeof alias === 'string') diff --git a/src/main/persistence/loading-store/normalize-loaded-state-collections.ts b/src/main/persistence/loading-store/normalize-loaded-state-collections.ts index 2b133d486b7..16c5b424def 100644 --- a/src/main/persistence/loading-store/normalize-loaded-state-collections.ts +++ b/src/main/persistence/loading-store/normalize-loaded-state-collections.ts @@ -41,11 +41,13 @@ export function normalizeLoadedLocalSession( export function normalizeLoadedHostSessions( parsed: PersistedState, defaults: PersistedState, + localSession: WorkspaceSessionState, markNeedsSave: () => void ): PersistedState['workspaceSessionsByHostId'] { const { partitions, repaired } = parseWorkspaceSessionsByHostId( parsed.workspaceSessionsByHostId, - defaults.workspaceSession + defaults.workspaceSession, + localSession ) if (repaired) { // Why: salvage repairs only the in-memory partitions; without a save the corrupt entries stay on disk and get re-dropped every launch. diff --git a/src/main/persistence/loading-store/primary-state-writes.ts b/src/main/persistence/loading-store/primary-state-writes.ts index 9d6e476ebe5..c112b4a9ba0 100644 --- a/src/main/persistence/loading-store/primary-state-writes.ts +++ b/src/main/persistence/loading-store/primary-state-writes.ts @@ -161,7 +161,8 @@ export async function writeToDiskAsync(owner: PrimaryStateWriteOperations): Prom // Why: fsync before rename, then fsync the directory; see writeFileDurable. const handle = await open(tmpFile, 'w') try { - await handle.writeFile(payload, 'utf-8') + // Already UTF-8 bytes: passing the string here would re-encode the whole state on the main thread. + await handle.writeFile(payload) await handle.sync() } finally { await handle.close() diff --git a/src/main/persistence/loading-store/secret-sentinel-substitution.test.ts b/src/main/persistence/loading-store/secret-sentinel-substitution.test.ts new file mode 100644 index 00000000000..e58d0280bae --- /dev/null +++ b/src/main/persistence/loading-store/secret-sentinel-substitution.test.ts @@ -0,0 +1,184 @@ +/** + * The bar for this change is "the bytes on disk did not move". Every case below runs the exact + * loop `applySecretSentinelSubstitutions` replaced β€” reproduced in `previousImplementation` β€” and + * compares payload bytes and guard hash, because a drifting hash silently disables the no-op write + * guard and a drifting payload is corrupted persisted state. + */ +import { createHash, randomUUID } from 'node:crypto' +import { describe, expect, it } from 'vitest' +import { + applySecretSentinelSubstitutions, + type SecretSentinelSubstitution +} from './secret-sentinel-substitution' + +/** Verbatim from state-serialization-secret-handling.ts before this change. */ +function previousImplementation( + serialized: string, + secretSubs: readonly SecretSentinelSubstitution[], + degradedPrefix: string +): { payload: Buffer; stateHash: string } { + let payload = serialized + let hashInput = serialized + for (const { sentinel, blob, hashValue } of secretSubs) { + const escapedSentinel = JSON.stringify(sentinel).slice(1, -1) + payload = payload.replace(escapedSentinel, () => JSON.stringify(blob).slice(1, -1)) + hashInput = hashInput.replace(escapedSentinel, () => JSON.stringify(hashValue).slice(1, -1)) + } + const stateHash = createHash('sha1').update(degradedPrefix).update(hashInput).digest('hex') + // `handle.writeFile(payload, 'utf-8')` is what turned the string into bytes. + return { payload: Buffer.from(payload, 'utf8'), stateHash } +} + +function expectIdenticalToPrevious( + serialized: string, + subs: readonly SecretSentinelSubstitution[], + degradedPrefix = '' +): void { + const before = previousImplementation(serialized, subs, degradedPrefix) + const after = applySecretSentinelSubstitutions(serialized, subs, degradedPrefix) + expect(after.payload.equals(before.payload)).toBe(true) + expect(after.stateHash).toBe(before.stateHash) +} + +function sentinel(): string { + return `orca-secret-slot-${randomUUID()}` +} + +describe('applySecretSentinelSubstitutions', () => { + it('produces bytes and a hash identical to the previous implementation', () => { + const subs: SecretSentinelSubstitution[] = [ + { sentinel: sentinel(), blob: 'djEwY2lwaGVy', hashValue: 'cookie-value' }, + { + sentinel: sentinel(), + // Regex-special *and* JSON-escapable, which is the pair that breaks a naive rewrite: + // `$&` would splice the match back in under string-form replace, and the backslash and + // quote have to survive `JSON.stringify(...).slice(1, -1)` unchanged. + blob: 'A+/=$&$1$`\\x "quoted" |.*?[](){}^', + hashValue: 'http://proxy.example:8080/?a=b&c=$&' + }, + { sentinel: sentinel(), blob: '', hashValue: 'https://kagi.com/session?t=abc' } + ] + const state = { + settings: { opencodeSessionCookie: subs[0].sentinel, httpProxyUrl: subs[1].sentinel }, + ui: { browserKagiSessionLink: subs[2].sentinel }, + // Adjacent content that must not shift: a near-miss prefix, and JSON escapes either side. + noise: ['orca-secret-slot-', 'a\\b"c\n\t', subs[0].sentinel.slice(0, -1)] + } + expectIdenticalToPrevious(JSON.stringify(state), subs) + }) + + it('stays identical when the state holds multi-byte and escaped characters', () => { + const subs: SecretSentinelSubstitution[] = [ + { sentinel: sentinel(), blob: 'blob-Γ©', hashValue: 'plain-Γ©' }, + { sentinel: sentinel(), blob: 'πŸ˜€', hashValue: 'δΈ­ζ–‡' } + ] + const state = { + // Segment boundaries land next to these, so a wrong split would corrupt the encode. + before: 'Γ©δΈ­ζ–‡πŸ˜€', + a: subs[0].sentinel, + between: 'πŸ˜€β€¨β€©', + b: subs[1].sentinel, + after: 'πŸ˜€' + } + expectIdenticalToPrevious(JSON.stringify(state), subs) + }) + + it('stays identical with no substitutions and with the degraded-storage prefix', () => { + const state = JSON.stringify({ settings: { httpProxyUrl: '' }, big: 'x'.repeat(4096) }) + expectIdenticalToPrevious(state, []) + expectIdenticalToPrevious(state, [], 'safeStorage-degraded\0') + + const subs = [{ sentinel: sentinel(), blob: 'b', hashValue: 'h' }] + expectIdenticalToPrevious( + JSON.stringify({ s: subs[0].sentinel }), + subs, + 'safeStorage-degraded\0' + ) + }) + + it('escapes regex metacharacters in the sentinel itself', () => { + // Not reachable from a UUID sentinel, but the alternation must not be able to become a pattern. + const subs = [{ sentinel: 'a.b*c(d)|e[f]', blob: 'BLOB', hashValue: 'HASH' }] + const serialized = JSON.stringify({ real: subs[0].sentinel, decoy: 'axbxxcXdX_eXfX' }) + expectIdenticalToPrevious(serialized, subs) + expect( + applySecretSentinelSubstitutions(serialized, subs, '').payload.toString('utf8') + ).toContain('axbxxcXdX_eXfX') + }) + + it('substitutes every occurrence when a sentinel repeats', () => { + // Cannot happen today (a sentinel is a UUID minted after the state is assembled, so it appears + // exactly once), but the old first-match-only `String.replace` would have written a raw + // sentinel to disk in place of a secret if it ever did. The alternation is global instead. + const subs = [{ sentinel: sentinel(), blob: 'CIPHER', hashValue: 'PLAIN' }] + const serialized = JSON.stringify({ a: subs[0].sentinel, b: subs[0].sentinel }) + const { payload } = applySecretSentinelSubstitutions(serialized, subs, '') + expect(payload.toString('utf8')).toBe(JSON.stringify({ a: 'CIPHER', b: 'CIPHER' })) + expect(payload.toString('utf8')).not.toContain(subs[0].sentinel) + }) + + it('copies and UTF-8 encodes the full state once, not once per sentinel per side', () => { + const subs: SecretSentinelSubstitution[] = Array.from({ length: 3 }, () => ({ + sentinel: sentinel(), + blob: 'CIPHERTEXT', + hashValue: 'plaintext' + })) + const serialized = JSON.stringify({ + pad: 'x'.repeat(200_000), + a: subs[0].sentinel, + b: subs[1].sentinel, + c: subs[2].sentinel + }) + const FULL_STATE = 100_000 + + // Both costs are observable at their sources: a `String.replace` whose receiver is the whole + // state allocates another copy of it, and every string handed to `Buffer.from` or `hash.update` + // is one full UTF-8 encode pass on the main thread. + const counted = (run: () => unknown): { fullStateReplaces: number; encodedChars: number } => { + const realReplace = String.prototype.replace + const realBufferFrom = Buffer.from + const hashProto = Object.getPrototypeOf(createHash('sha1')) as { + update: (...args: unknown[]) => unknown + } + const realUpdate = hashProto.update + const counts = { fullStateReplaces: 0, encodedChars: 0 } + String.prototype.replace = function (this: string, ...args: unknown[]) { + if (this.length >= FULL_STATE) { + counts.fullStateReplaces++ + } + return realReplace.apply(this, args as never) + } as typeof String.prototype.replace + Buffer.from = function (...args: unknown[]) { + if (typeof args[0] === 'string') { + counts.encodedChars += args[0].length + } + return (realBufferFrom as (...a: unknown[]) => Buffer).apply(Buffer, args) + } as typeof Buffer.from + hashProto.update = function (this: unknown, ...args: unknown[]) { + if (typeof args[0] === 'string') { + counts.encodedChars += args[0].length + } + return realUpdate.apply(this, args) + } + try { + run() + } finally { + String.prototype.replace = realReplace + Buffer.from = realBufferFrom + hashProto.update = realUpdate + } + return counts + } + + const before = counted(() => previousImplementation(serialized, subs, '')) + const after = counted(() => applySecretSentinelSubstitutions(serialized, subs, '')) + + // Two `String.replace` calls over the whole state per sentinel β€” payload and hash input. + expect(before.fullStateReplaces).toBe(subs.length * 2) + expect(after.fullStateReplaces).toBe(0) + // The old path encoded the state twice: once for sha1, once for the file write. + expect(before.encodedChars).toBeGreaterThan(serialized.length * 1.9) + expect(after.encodedChars).toBeLessThan(serialized.length * 1.1) + expect(after.encodedChars).toBeGreaterThan(serialized.length * 0.9) + }) +}) diff --git a/src/main/persistence/loading-store/secret-sentinel-substitution.ts b/src/main/persistence/loading-store/secret-sentinel-substitution.ts new file mode 100644 index 00000000000..afcdddafa77 --- /dev/null +++ b/src/main/persistence/loading-store/secret-sentinel-substitution.ts @@ -0,0 +1,78 @@ +import { createHash } from 'node:crypto' +import { escapeRegex } from '../../../shared/string-utils' + +export type SecretSentinelSubstitution = { + /** The `orca-secret-slot-` placeholder standing in the serialized state. */ + sentinel: string + /** What the on-disk payload gets: the ciphertext. */ + blob: string + /** What the guard hash gets: a value stable across non-deterministic encryption. */ + hashValue: string +} + +/** + * Replace every secret sentinel in `serialized` in ONE pass, producing the on-disk bytes and the + * guard hash from the same encoded segments. + * + * Why not the obvious `payload.replace(...)` / `hashInput.replace(...)` loop it replaces: each + * `String.replace` returns a rope that the *next* `replace` has to flatten before it can search, so + * N sentinels cost 2N-1 flattened copies of the whole multi-MB state, plus one more per side when + * `hash.update` and the file write finally consume them. Measured on a 4.65 MB store with three + * sentinels: 7 full-state string allocations, 62 MB of V8 heap, 27 MB of it in large_object_space. + * + * Here the state is walked once, each literal run is UTF-8 encoded exactly once, and those same + * buffers feed both the payload and the hash β€” 1 full-state string, 1 encode. + * + * Byte-for-byte identical output to the loop: both sides read the sentinel in its JSON-escaped + * form, the replacements are the JSON-escaped `blob`/`hashValue`, and the hash sees the same byte + * sequence it saw when it was handed one concatenated string. + */ +export function applySecretSentinelSubstitutions( + serialized: string, + substitutions: readonly SecretSentinelSubstitution[], + degradedPrefix: string +): { payload: Buffer; stateHash: string } { + const hash = createHash('sha1').update(degradedPrefix) + if (substitutions.length === 0) { + const payload = Buffer.from(serialized, 'utf8') + return { payload, stateHash: hash.update(payload).digest('hex') } + } + + const replacementBySentinel = new Map() + const alternatives: string[] = [] + for (const { sentinel, blob, hashValue } of substitutions) { + // Preserved from the loop this replaces: both the search key and the replacements are the + // JSON-escaped forms, because that is what `serialized` actually contains. + const escapedSentinel = JSON.stringify(sentinel).slice(1, -1) + if (replacementBySentinel.has(escapedSentinel)) { + continue + } + alternatives.push(escapeRegex(escapedSentinel)) + replacementBySentinel.set(escapedSentinel, { + blob: Buffer.from(JSON.stringify(blob).slice(1, -1), 'utf8'), + hashValue: Buffer.from(JSON.stringify(hashValue).slice(1, -1), 'utf8') + }) + } + + // Global, though a sentinel is a UUID minted after the state was assembled and so occurs exactly + // once: a single pass that substitutes every occurrence cannot leave one behind on disk. + const pattern = new RegExp(alternatives.join('|'), 'g') + const chunks: Buffer[] = [] + let cursor = 0 + let match: RegExpExecArray | null + while ((match = pattern.exec(serialized)) !== null) { + // Non-null: the alternation is built from exactly the map's keys. + const replacement = replacementBySentinel.get(match[0])! + // A sliced substring, so this does not copy the state; the encode below is its only pass. + const literal = Buffer.from(serialized.slice(cursor, match.index), 'utf8') + chunks.push(literal, replacement.blob) + hash.update(literal) + hash.update(replacement.hashValue) + cursor = match.index + match[0].length + } + const tail = Buffer.from(serialized.slice(cursor), 'utf8') + chunks.push(tail) + hash.update(tail) + + return { payload: Buffer.concat(chunks), stateHash: hash.digest('hex') } +} diff --git a/src/main/persistence/loading-store/state-serialization-secret-handling.ts b/src/main/persistence/loading-store/state-serialization-secret-handling.ts index c9d81f071a5..61689ea0f13 100644 --- a/src/main/persistence/loading-store/state-serialization-secret-handling.ts +++ b/src/main/persistence/loading-store/state-serialization-secret-handling.ts @@ -1,4 +1,4 @@ -import { createHash, randomUUID } from 'node:crypto' +import { randomUUID } from 'node:crypto' import type { PersistedState } from '../../../shared/persisted-state-types' import { collectFolderWorkspaceDiffComments } from '../../folder-workspace-diff-comments' import { @@ -8,6 +8,10 @@ import { } from '../../protected-secret-persistence' import { stripRetiredGlobalSettings } from '../applying-settings/terminal-settings-migrations' +import { + applySecretSentinelSubstitutions, + type SecretSentinelSubstitution +} from './secret-sentinel-substitution' import type { StoreRuntimeState } from './store-runtime-state' type StateSerializationSecretHandlingOperationsRuntime = Pick< @@ -24,7 +28,7 @@ export class StateSerializationSecretHandlingOperations { } buildStateToSave(): { - payload: string + payload: Buffer stateHash: string protectedSecretUpdates: ProtectedSecretRetentionUpdate[] } { @@ -37,7 +41,7 @@ export class StateSerializationSecretHandlingOperations { // on deterministic-IV platforms (macOS/legacy-Linux OSCrypt). A per-slot // random UUID can't occur anywhere else in the serialized state (the user // sets their data before it is minted), so it appears exactly once. - const secretSubs: { sentinel: string; blob: string; hashValue: string }[] = [] + const secretSubs: SecretSentinelSubstitution[] = [] const protectedSecretUpdates: ProtectedSecretRetentionUpdate[] = [] let protectedStorageDegraded = false const encryptToSentinel = (slot: string, plaintext: string): string => { @@ -105,21 +109,14 @@ export class StateSerializationSecretHandlingOperations { // Why compact: ~20% fewer bytes and less serialize time; all readers JSON.parse so formatting is irrelevant. // One full-state stringify; secret slots currently hold sentinels. const serialized = JSON.stringify(stateToSave) - // Substitute each unique sentinel exactly once: ciphertext for the on-disk - // payload, a stable normalized value for the guard hash. Function-form - // replacement keeps `$` inert; both sides read the sentinel as JSON-escaped - // in `serialized`, so each replace is byte-for-byte position-exact. - let payload = serialized - let hashInput = serialized - for (const { sentinel, blob, hashValue } of secretSubs) { - const escapedSentinel = JSON.stringify(sentinel).slice(1, -1) - payload = payload.replace(escapedSentinel, () => JSON.stringify(blob).slice(1, -1)) - hashInput = hashInput.replace(escapedSentinel, () => JSON.stringify(hashValue).slice(1, -1)) - } - const stateHash = createHash('sha1') - .update(protectedStorageDegraded ? 'safeStorage-degraded\0' : '') - .update(hashInput) - .digest('hex') + // Substitute each unique sentinel: ciphertext for the on-disk payload, a stable normalized + // value for the guard hash. One pass builds both, so the multi-MB state is never copied per + // sentinel and never encoded twice. + const { payload, stateHash } = applySecretSentinelSubstitutions( + serialized, + secretSubs, + protectedStorageDegraded ? 'safeStorage-degraded\0' : '' + ) return { payload, stateHash, protectedSecretUpdates } } } diff --git a/src/main/persistence/loading-store/state-write-round-trip.test.ts b/src/main/persistence/loading-store/state-write-round-trip.test.ts new file mode 100644 index 00000000000..ee7f26feb3e --- /dev/null +++ b/src/main/persistence/loading-store/state-write-round-trip.test.ts @@ -0,0 +1,131 @@ +/** + * The write path now hands the file a Buffer it built in one pass instead of a string it rebuilt + * per secret. Drives the real `Store` end to end β€” encrypted settings, a local session and a remote + * host partition β€” and reloads from the file it actually wrote, because the failure this guards + * against (a mis-sliced segment, a re-encoded payload, a dropped sentinel) is invisible until + * something reads the bytes back. + */ +import { mkdtempSync, readFileSync, realpathSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' + +vi.mock('electron', () => ({ + app: { + getPath: () => tmpdir(), + getName: () => 'orca-test', + getVersion: () => '0.0.0-test', + isPackaged: false, + on: () => {}, + whenReady: () => Promise.resolve() + }, + safeStorage: { + // Encryption ON, so the secret slots really do mint sentinels and the substitution pass runs. + isEncryptionAvailable: () => true, + encryptString: (value: string) => Buffer.from(`enc:${value}`), + decryptString: (value: Buffer) => value.toString().slice(4) + }, + ipcMain: { on: () => {}, handle: () => {} }, + BrowserWindow: { getAllWindows: () => [] } +})) + +const { Store } = await import('./store') + +const HOST_ID = 'ssh:user@host' + +const stores: InstanceType[] = [] +afterEach(() => { + for (const store of stores.splice(0)) { + store.flush() + } + vi.restoreAllMocks() +}) + +function openStore(dataFile: string): InstanceType { + const store = new Store({ dataFile }) + stores.push(store) + return store +} + +function session(activeTabId: string): WorkspaceSessionState { + return { + activeRepoId: 'repo-1', + // Left null: the load path's deregistered-repo sweep nulls an active worktree whose repo is + // not registered, which would mask what this test is actually about. + activeWorktreeId: null, + activeTabId, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + // Non-ASCII on purpose: a byte-offset mistake in the encode shows up here first. + browserUrlHistory: [ + { + url: 'https://example.test/Γ©πŸ˜€', + normalizedUrl: 'https://example.test/Γ©πŸ˜€', + title: 'δΈ­ζ–‡ title', + lastVisitedAt: 17, + visitCount: 3 + } + ] + } as WorkspaceSessionState +} + +describe('persisted state survives a save/load round trip', () => { + it('reloads settings, secrets and both session partitions unchanged', () => { + const dataFile = join( + realpathSync(mkdtempSync(join(tmpdir(), 'orca-store-round-trip-'))), + 'orca-data.json' + ) + const written = openStore(dataFile) + written.updateSettings({ + // Three secret slots, i.e. three sentinels in one save β€” the case the old loop paid 7 copies for. + opencodeSessionCookie: 'cookie-Γ©-value', + httpProxyUrl: 'http://proxy.example:8080/?a=b&c=$&' + }) + written.updateUI({ browserKagiSessionLink: 'https://kagi.com/session?t=abc' }) + written.setWorkspaceSession(session('local-tab')) + written.setWorkspaceSession(session('remote-tab'), HOST_ID) + written.flush() + + const before = { + settings: written.getSettings(), + ui: written.getUI(), + local: written.getWorkspaceSession(), + remote: written.getWorkspaceSession(HOST_ID) + } + + // The file is valid UTF-8 JSON and holds ciphertext, not the plaintext secrets. + const bytes = readFileSync(dataFile) + const onDisk = JSON.parse(bytes.toString('utf8')) + expect(onDisk.settings.opencodeSessionCookie).not.toBe('cookie-Γ©-value') + expect(Buffer.from(onDisk.settings.opencodeSessionCookie, 'base64').toString('utf8')).toContain( + 'cookie-Γ©-value' + ) + expect(bytes.toString('utf8')).not.toContain('orca-secret-slot-') + + const reloaded = openStore(dataFile) + expect(reloaded.getSettings().opencodeSessionCookie).toBe(before.settings.opencodeSessionCookie) + expect(reloaded.getSettings().httpProxyUrl).toBe(before.settings.httpProxyUrl) + expect(reloaded.getUI().browserKagiSessionLink).toBe(before.ui.browserKagiSessionLink) + // `toMatchObject`: the load path spreads session defaults over what was written, so the + // reloaded slice is a superset. Exact deep equality is asserted on the second trip below. + expect(reloaded.getWorkspaceSession()).toMatchObject(before.local) + // The remote partition keeps everything it owns; only globals local already holds are dropped, + // and `browserUrlHistory` comes back at its default from the same spread as before. + expect(reloaded.getWorkspaceSession(HOST_ID).activeTabId).toBe('remote-tab') + expect(reloaded.getWorkspaceSession(HOST_ID).browserUrlHistory).toEqual([]) + + // Deep equality of the whole reloaded state, taken across a second round trip so the assertion + // is not comparing against the first load's one-time settings migrations. + reloaded.flush() + const bytesAfterReload = readFileSync(dataFile) + const again = openStore(dataFile) + expect(again.getSettings()).toEqual(reloaded.getSettings()) + expect(again.getUI()).toEqual(reloaded.getUI()) + expect(again.getWorkspaceSession()).toEqual(reloaded.getWorkspaceSession()) + expect(again.getWorkspaceSession(HOST_ID)).toEqual(reloaded.getWorkspaceSession(HOST_ID)) + // ...and the bytes are stable, so a quiet app is not rewriting a 4 MB file with new content. + again.flush() + expect(readFileSync(dataFile).equals(bytesAfterReload)).toBe(true) + }) +}) diff --git a/src/main/persistence/loading-store/workspace-session-partitions.test.ts b/src/main/persistence/loading-store/workspace-session-partitions.test.ts new file mode 100644 index 00000000000..f9697f77997 --- /dev/null +++ b/src/main/persistence/loading-store/workspace-session-partitions.test.ts @@ -0,0 +1,141 @@ +/** + * Global session fields live in the 'local' slice. Copies of them inside a non-local host partition + * are legacy residue: the split never writes them there and the merge never reads them from there + * unless local has nothing. These tests pin the drop to exactly that condition, keep the renderer's + * merge landing on the same value either way, and re-check the two safety gates that decide which + * global fields may be dropped at all. + */ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import type { BrowserHistoryEntry } from '../../../shared/browser-workspace-types' +import type { WorkspaceDocHistoryEntry } from '../../../shared/workspace-doc-history' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' +import { WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND } from '../restoring-sessions/session-worktree-ownership' +import { + HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS, + parseWorkspaceSessionsByHostId +} from './workspace-session-partitions' + +const HOST = 'ssh:target-1' + +function history(url: string): BrowserHistoryEntry[] { + return [{ url, normalizedUrl: url, title: url, lastVisitedAt: 1, visitCount: 1 }] +} + +function docEntry(filePath: string): WorkspaceDocHistoryEntry { + return { + docLocation: { kind: 'workspace-doc', worktreeId: 'repo-1::/tmp/a', filePath }, + title: filePath, + lastVisitedAt: 2, + visitCount: 1 + } +} + +function localSession(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +function parse( + raw: Record, + local?: WorkspaceSessionState +): Partial> { + return parseWorkspaceSessionsByHostId(raw, getDefaultWorkspaceSession(), local).partitions +} + +describe('HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS', () => { + it('only lists fields that are global AND that no worktree-ownership pass follows', () => { + for (const field of HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS) { + // Gate 1: the renderer's split/merge treat it as local-owned, so a non-local copy is dead. + expect(WORKSPACE_SESSION_FIELD_OWNERSHIP[field]).toBe('global') + // Gate 2: `collectPersistedSessionWorktreeOwners` and the deregistered-repo residue sweep + // walk EVERY partition through this table. Anything but 'none' means dropping the field + // could un-own a worktree and get its metadata pruned. + expect(WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND[field]).toBe('none') + } + }) +}) + +describe('parseWorkspaceSessionsByHostId global-field residue', () => { + it('drops a non-local global field the local slice already owns', () => { + const local = localSession({ browserUrlHistory: history('https://local.test') }) + const partitions = parse( + { + [HOST]: { + ...getDefaultWorkspaceSession(), + browserUrlHistory: history('https://stale.test') + } + }, + local + ) + // Back to the default from the spread, not the 65 KB stale replica. The merge reads this field + // from local whenever local has it, so the renderer still sees `https://local.test` + // (`workspace-session-host-split.test.ts` pins that half of the contract). + expect(partitions[HOST]?.browserUrlHistory).toEqual([]) + }) + + it('retains a non-local global field the local slice does NOT have', () => { + // `workspaceDocHistory` is optional and absent from the defaults, so local can genuinely lack + // it and the merge's fallback to another slice is live. + const local = localSession({}) + expect(local.workspaceDocHistory).toBeUndefined() + const docs = [docEntry('/repo/remote.md')] + const partitions = parse( + { [HOST]: { ...getDefaultWorkspaceSession(), workspaceDocHistory: docs } }, + local + ) + // Retained, so the merge's "fall back to any slice that has it" path still finds a value. + expect(partitions[HOST]?.workspaceDocHistory).toEqual(docs) + }) + + it('drops that same field once the local slice does have it', () => { + const localDocs = [docEntry('/repo/local.md')] + const local = localSession({ workspaceDocHistory: localDocs }) + const partitions = parse( + { + [HOST]: { + ...getDefaultWorkspaceSession(), + workspaceDocHistory: [docEntry('/repo/stale.md')] + } + }, + local + ) + expect(partitions[HOST]).not.toHaveProperty('workspaceDocHistory') + expect(local.workspaceDocHistory).toEqual(localDocs) + }) + + it('leaves worktree-referencing globals and host-owned fields alone', () => { + const local = localSession({ + browserUrlHistory: history('https://local.test'), + activeWorktreeId: 'repo-1::/tmp/local', + activeTabId: 'local-tab' + }) + const tabs = { 'repo-1::/tmp/a': [] } + const partitions = parse( + { + [HOST]: { + ...getDefaultWorkspaceSession(), + // A `'direct'` worktree reference the residue sweep reads out of every partition. + activeWorktreeId: 'repo-1::/tmp/a', + // Read on a partition by the mobile terminal projection. + activeTabId: 'remote-tab', + tabsByWorktree: tabs, + terminalTopologyRevisionByRepoId: { 'repo-1': 4 } + } + }, + local + ) + expect(partitions[HOST]?.activeWorktreeId).toBe('repo-1::/tmp/a') + expect(partitions[HOST]?.activeTabId).toBe('remote-tab') + expect(partitions[HOST]?.tabsByWorktree).toEqual(tabs) + expect(partitions[HOST]?.terminalTopologyRevisionByRepoId).toEqual({ 'repo-1': 4 }) + }) + + it('is a no-op when no local slice is supplied', () => { + const stale = history('https://stale.test') + const partitions = parse({ + [HOST]: { ...getDefaultWorkspaceSession(), browserUrlHistory: stale } + }) + expect(partitions[HOST]?.browserUrlHistory).toEqual(stale) + }) +}) diff --git a/src/main/persistence/loading-store/workspace-session-partitions.ts b/src/main/persistence/loading-store/workspace-session-partitions.ts index 50e9d4ae8cc..b6f78479dbf 100644 --- a/src/main/persistence/loading-store/workspace-session-partitions.ts +++ b/src/main/persistence/loading-store/workspace-session-partitions.ts @@ -17,11 +17,49 @@ export function workspaceSessionSalvageLogDetails(result: { } } +/** + * Global fields belong to the 'local' slice: the split writes them only there and the merge reads + * them only from there. A copy inside a non-local partition is legacy residue no read can reach β€” + * stale `browserUrlHistory` replicas alone were 589 KB, 12.7% of a 4.65 MB store, rewritten on + * every save and reparsed on every launch. + * + * Deliberately NOT every field in `GLOBAL_WORKSPACE_SESSION_FIELDS`. Two separate gates disqualify + * the rest, and both are load-bearing: + * - `activeWorktreeId` and `activeWorkspaceKey` are `'direct'` in + * `WORKSPACE_SESSION_WORKTREE_REFERENCE_KIND`, and both `collectPersistedSessionWorktreeOwners` + * and the deregistered-repo residue sweep read them out of EVERY partition. Dropping one + * un-owns a worktree, and an un-owned worktree gets its metadata pruned. + * - `activeTabId`, `activeConnectionIdsAtShutdown` and `activeRepoId` have live main-side readers + * on a partition: `isPersistedTerminalLeafActive` falls back to `activeTabId` for the mobile + * projection, and the runtime attach-window handoff unions `activeConnectionIdsAtShutdown`. + * + * `workspace-session-partitions.test.ts` re-checks both gates for every field listed here. + */ +export const HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS = [ + 'browserUrlHistory', + 'workspaceDocHistory' +] as const satisfies readonly (keyof WorkspaceSessionState)[] + +/** Dropped only where local already holds the field β€” exactly when the merge's fallback to another + * slice cannot fire. Runs before the defaults spread, so a field the type requires comes back at + * its default rather than going missing. */ +function dropRedundantGlobalFields( + slice: Partial, + local: WorkspaceSessionState | undefined +): void { + for (const field of HOST_PARTITION_REDUNDANT_GLOBAL_FIELDS) { + if (local?.[field] !== undefined) { + delete slice[field] + } + } +} + /** Normalize non-'local' host partitions; 'local' (the legacy workspaceSession blob) is dropped so the two surfaces never diverge. * Each partition is zod-validated independently, so one corrupt host drops to defaults without taking out the others. Idempotent. */ export function parseWorkspaceSessionsByHostId( raw: unknown, - defaults: WorkspaceSessionState + defaults: WorkspaceSessionState, + localSession?: WorkspaceSessionState ): { partitions: Partial>; repaired: boolean } { if (!raw || typeof raw !== 'object' || Array.isArray(raw)) { return { partitions: {}, repaired: raw !== undefined } @@ -50,6 +88,7 @@ export function parseWorkspaceSessionsByHostId( ) repaired = true } + dropRedundantGlobalFields(result.value, localSession) partitions[hostId] = { ...defaults, ...result.value } } return { partitions, repaired } diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index 51d09027200..652678e24a1 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -10,7 +10,7 @@ import { getWorktreeIdFromHostIdentity, isWorktreeHostIdentity } from '../../../shared/worktree/host-qualified-identity' -import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from './workspace-session-host-field-ownership' +import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' import { isWorkspaceSessionRecord, type WorkspaceSessionRecord 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 de359cfb12e..ad540d2af04 100644 --- a/src/renderer/src/lib/workspace-session-host-split.test.ts +++ b/src/renderer/src/lib/workspace-session-host-split.test.ts @@ -392,3 +392,46 @@ describe('split β†’ merge round trip', () => { expect(roundTrip(state)).toEqual(state) }) }) + +/** + * The main-process load path drops a global field from a non-local partition when the local slice + * already has it, on the strength of exactly these two rules. If either moves, that prune starts + * discarding a value the renderer would otherwise have read. + */ +describe('mergeWorkspaceSessionsFromHosts global-field precedence', () => { + const localEntry = { + url: 'local', + normalizedUrl: 'local', + title: 'l', + lastVisitedAt: 2, + visitCount: 1 + } + const hostEntry = { + url: 'host', + normalizedUrl: 'host', + title: 'h', + lastVisitedAt: 1, + visitCount: 1 + } + + it("takes a global field from 'local' whenever local has one, ignoring every other slice", () => { + const merged = mergeWorkspaceSessionsFromHosts({ + [LOCAL_EXECUTION_HOST_ID]: { + ...getDefaultWorkspaceSession(), + browserUrlHistory: [localEntry] + }, + [RUNTIME_A]: { ...getDefaultWorkspaceSession(), browserUrlHistory: [hostEntry] } + }) + expect(merged.browserUrlHistory).toEqual([localEntry]) + }) + + it('falls back to another slice only when local does not have the field', () => { + const local = getDefaultWorkspaceSession() + delete local.browserUrlHistory + const merged = mergeWorkspaceSessionsFromHosts({ + [LOCAL_EXECUTION_HOST_ID]: local, + [RUNTIME_A]: { ...getDefaultWorkspaceSession(), browserUrlHistory: [hostEntry] } + }) + expect(merged.browserUrlHistory).toEqual([hostEntry]) + }) +}) diff --git a/src/renderer/src/lib/workspace-session-host-split.ts b/src/renderer/src/lib/workspace-session-host-split.ts index 817b87eebeb..ea03cc6f3f8 100644 --- a/src/renderer/src/lib/workspace-session-host-split.ts +++ b/src/renderer/src/lib/workspace-session-host-split.ts @@ -8,7 +8,7 @@ import { isWorktreeHostIdentity } from '../../../shared/worktree/host-qualified- import { GLOBAL_WORKSPACE_SESSION_FIELDS, WORKSPACE_SESSION_FIELD_OWNERSHIP -} from './workspace-session-host-field-ownership' +} from '../../../shared/workspace-session-host-field-ownership' import { buildWorktreeIdByFileId, buildWorktreeIdByTabId, diff --git a/src/renderer/src/lib/workspace-session-host-field-ownership.ts b/src/shared/workspace-session-host-field-ownership.ts similarity index 97% rename from src/renderer/src/lib/workspace-session-host-field-ownership.ts rename to src/shared/workspace-session-host-field-ownership.ts index a44c7669bda..6397c2ad239 100644 --- a/src/renderer/src/lib/workspace-session-host-field-ownership.ts +++ b/src/shared/workspace-session-host-field-ownership.ts @@ -1,4 +1,4 @@ -import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { WorkspaceSessionState } from './workspace-session-state-types' export type WorkspaceSessionFieldOwnership = | 'global'