mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix: preserve paired host sessions during startup residue cleanup (#18922)
* fix: preserve paired host sessions during startup residue cleanup
* refactor(persistence): tighten the paired-host retention pass
Dedupe the owner-key -> repo-id extraction the retention and seeding
passes both needed, and name the `runtime:*` check instead of repeating
the parse three times.
Reach the session walker directly by exporting
`addWorkspaceSessionWorktreeOwners` rather than fabricating a
`{ workspaceSession }` state slice to get at it.
Correct the docstrings: `runtime:*` also covers a serving host's own
partition, and the "authoritative removal" they promised has no product
caller on a paired client today, so say what the exemption actually
costs.
Add a survived-load assertion to the explicit-removal test, which
otherwise passed against the pre-fix sweep -- the partition was already
empty before the removal ran.
No behavior change beyond the docs and the test assertion.
This commit is contained in:
@@ -1,7 +1,5 @@
|
||||
// Why this file exists: deregistering a project used to strand every row it owned. No sweeper could
|
||||
// reach them -- the missing-directory prune is gated on the repo still being registered, and a
|
||||
// paired client's mirror of a remote host's rows is keyed by ids that client never registers, so the
|
||||
// owning host's removal never reached it (#17776).
|
||||
// reach them because the missing-directory prune is gated on the repo still being registered.
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
|
||||
import { rmSync, mkdtempSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
@@ -103,7 +101,7 @@ describe('deregistered repo residue', () => {
|
||||
expect(session.sleepingAgentSessionsByPaneKey ?? {}).toEqual({})
|
||||
})
|
||||
|
||||
it("sweeps a remote host's session partition the owning host's removal can never reach", async () => {
|
||||
it('keeps a remote session whose repo is not registered on the desktop', async () => {
|
||||
writeDataFile({
|
||||
schemaVersion: 1,
|
||||
repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })],
|
||||
@@ -117,8 +115,10 @@ describe('deregistered repo residue', () => {
|
||||
store.flush()
|
||||
|
||||
const partition = store.getWorkspaceSession(RUNTIME_HOST)
|
||||
expect(partition.tabsByWorktree).toEqual({})
|
||||
expect(partition.activeTabTypeByWorktree).toEqual({})
|
||||
expect(partition.tabsByWorktree[GONE_WORKTREE]).toHaveLength(1)
|
||||
expect(partition.activeTabTypeByWorktree).toEqual(
|
||||
sessionFor(GONE_WORKTREE).activeTabTypeByWorktree
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps rows for every registered repo, on any execution host', async () => {
|
||||
@@ -204,15 +204,13 @@ describe('deregistered repo residue', () => {
|
||||
schemaVersion: 1,
|
||||
repos: [makeRepo({ id: LIVE_REPO, path: '/workspace/live' })],
|
||||
worktreeMeta: {},
|
||||
workspaceSessionsByHostId: {
|
||||
[RUNTIME_HOST]: { ...getDefaultWorkspaceSession(), ...session }
|
||||
}
|
||||
workspaceSession: { ...getDefaultWorkspaceSession(), ...session }
|
||||
})
|
||||
|
||||
const store = await createStore()
|
||||
store.flush()
|
||||
|
||||
const partition = store.getWorkspaceSession(RUNTIME_HOST)
|
||||
const partition = store.getWorkspaceSession()
|
||||
expect(partition.activeWorktreeId ?? null).toBeNull()
|
||||
expect(partition.activeWorkspaceKey ?? null).toBeNull()
|
||||
expect(partition.activeWorktreeIdsOnShutdown ?? []).toEqual([])
|
||||
|
||||
@@ -0,0 +1,109 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { getDefaultWorkspaceSession } from '../shared/constants'
|
||||
import type { BrowserPage, BrowserWorkspace } from '../shared/browser-workspace-types'
|
||||
import { createStore, makeRepo, testState } from './persistence-test-harness'
|
||||
|
||||
vi.mock('./ssh/ssh-config-parser', () => ({
|
||||
loadUserSshConfig: vi.fn(),
|
||||
sshConfigHostsToTargets: vi.fn()
|
||||
}))
|
||||
vi.mock('electron', () => ({
|
||||
app: { getPath: () => testState.dir },
|
||||
safeStorage: { isEncryptionAvailable: () => false }
|
||||
}))
|
||||
vi.mock('./telemetry/client', () => ({ track: vi.fn() }))
|
||||
vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn().mockReturnValue({}) }))
|
||||
|
||||
const HOST = 'runtime:paired-host'
|
||||
const REPO = 'remote-repo'
|
||||
const WORKTREE = `${REPO}::/remote/project`
|
||||
const PAGE: BrowserPage = {
|
||||
id: 'page-1',
|
||||
workspaceId: 'browser-1',
|
||||
worktreeId: WORKTREE,
|
||||
url: 'https://example.test/moved',
|
||||
title: 'Moved page',
|
||||
loading: false,
|
||||
canGoBack: true,
|
||||
canGoForward: false,
|
||||
faviconUrl: null,
|
||||
loadError: null,
|
||||
createdAt: 1,
|
||||
browserRuntimeEnvironmentId: 'paired-host',
|
||||
remoteBrowserPageId: 'remote-page-1',
|
||||
remoteBrowserPageClientHosted: true
|
||||
}
|
||||
const BROWSER: BrowserWorkspace = {
|
||||
id: PAGE.workspaceId,
|
||||
worktreeId: WORKTREE,
|
||||
sessionProfileId: null,
|
||||
activePageId: PAGE.id,
|
||||
pageIds: [PAGE.id],
|
||||
url: PAGE.url,
|
||||
title: PAGE.title,
|
||||
loading: false,
|
||||
faviconUrl: null,
|
||||
canGoBack: true,
|
||||
canGoForward: false,
|
||||
loadError: null,
|
||||
createdAt: 1
|
||||
}
|
||||
|
||||
function browserSession() {
|
||||
return {
|
||||
...getDefaultWorkspaceSession(),
|
||||
browserTabsByWorktree: { [WORKTREE]: [BROWSER] },
|
||||
browserPagesByWorkspace: { [BROWSER.id]: [PAGE] },
|
||||
activeBrowserTabIdByWorktree: { [WORKTREE]: BROWSER.id }
|
||||
}
|
||||
}
|
||||
|
||||
describe('remote session startup ownership', () => {
|
||||
beforeEach(() => {
|
||||
testState.dir = mkdtempSync(join(tmpdir(), 'orca-remote-session-'))
|
||||
})
|
||||
afterEach(() => {
|
||||
rmSync(testState.dir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it('keeps a paired browser row and its hosting identity across two Store reloads', () => {
|
||||
const seed = createStore()
|
||||
seed.addRepo(makeRepo({ id: 'local-repo', path: join(testState.dir, 'local') }))
|
||||
seed.setWorkspaceSession(browserSession(), HOST)
|
||||
seed.flush()
|
||||
|
||||
for (let i = 0; i < 2; i += 1) {
|
||||
const reloaded = createStore()
|
||||
expect(reloaded.getWorkspaceSession(HOST).browserPagesByWorkspace).toEqual({
|
||||
[BROWSER.id]: [PAGE]
|
||||
})
|
||||
expect(reloaded.sweepDeregisteredRepoResidue()).toEqual([])
|
||||
reloaded.flush()
|
||||
}
|
||||
})
|
||||
|
||||
it('retains remote metadata when no session or local catalog row names its repo', () => {
|
||||
const seed = createStore()
|
||||
seed.setWorktreeMetaForHost(WORKTREE, HOST, { displayName: 'Remote work' })
|
||||
seed.flush()
|
||||
const reloaded = createStore()
|
||||
expect(reloaded.getWorktreeMeta(WORKTREE)).toMatchObject({ displayName: 'Remote work' })
|
||||
expect(reloaded.sweepDeregisteredRepoResidue()).toEqual([])
|
||||
reloaded.flush()
|
||||
})
|
||||
|
||||
it('still applies an explicit remote project removal', () => {
|
||||
const seed = createStore()
|
||||
seed.setWorkspaceSession(browserSession(), HOST)
|
||||
seed.flush()
|
||||
const reloaded = createStore()
|
||||
// Assert the row survived load first, or an empty partition below would prove nothing.
|
||||
expect(reloaded.getWorkspaceSession(HOST).browserPagesByWorkspace).not.toEqual({})
|
||||
reloaded.removeProjectForHost(REPO, HOST)
|
||||
reloaded.flush()
|
||||
expect(createStore().getWorkspaceSession(HOST).browserPagesByWorkspace).toEqual({})
|
||||
})
|
||||
})
|
||||
@@ -136,11 +136,9 @@ export class RepoLifecycleOperations {
|
||||
/**
|
||||
* Drop every persisted row owned by a repo id that is no longer registered.
|
||||
*
|
||||
* Runs at load because no removal path can: `removeProject` only fires while the repo is still in
|
||||
* `state.repos`, and a paired client's mirror of a remote host's rows is keyed by ids that client
|
||||
* never registers, so the owning host's removal never reaches it (#17776). An orphan has no owner
|
||||
* that could object, so this ignores the session-ownership and local-execution-host gates the
|
||||
* missing-directory sweeper needs.
|
||||
* Runs at load to reach leftover local rows after deregistration. Rows owned by a `runtime:*`
|
||||
* host are exempt: this runs before pairing, so their absence from the local catalog cannot
|
||||
* establish deletion. Only an explicit `removeProjectForHost` retires them.
|
||||
*/
|
||||
sweepDeregisteredRepoResidue(): string[] {
|
||||
const state = this[repoLifecycleOperationsContext].runtime.state
|
||||
|
||||
@@ -209,7 +209,7 @@ export function collectWorkspaceSessionWorktreeOwners(
|
||||
return owners
|
||||
}
|
||||
|
||||
function addWorkspaceSessionWorktreeOwners(
|
||||
export function addWorkspaceSessionWorktreeOwners(
|
||||
session: WorkspaceSessionState,
|
||||
collector: WorktreeOwnerCandidateCollector
|
||||
): void {
|
||||
|
||||
@@ -1,19 +1,61 @@
|
||||
import type { PersistedState } from '../../../shared/persisted-state-types'
|
||||
import { getWorktreeIdFromHostIdentity } from '../../../shared/worktree/host-qualified-identity'
|
||||
import {
|
||||
getExecutionHostIdFromWorktreeHostIdentity,
|
||||
getWorktreeIdFromHostIdentity
|
||||
} from '../../../shared/worktree/host-qualified-identity'
|
||||
import { parseExecutionHostId } from '../../../shared/execution-host'
|
||||
import { addWorkspaceSessionWorktreeOwners } from '../restoring-sessions/session-worktree-ownership'
|
||||
import { splitWorktreeId } from '../../../shared/worktree/id'
|
||||
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
|
||||
import { SESSION_FIELDS_PRUNED_BY_OWNER_KEY } from '../../orca-profiles/profile-project-session-field-disposition'
|
||||
import { ownerKeyWorktreeIds } from '../../orca-profiles/profile-project-worktree-identity'
|
||||
|
||||
/** A `runtime:*` host addresses a paired Orca desktop's rows, whose catalog lives on that host. */
|
||||
const isPairedHost = (hostId: string | null | undefined): boolean =>
|
||||
parseExecutionHostId(hostId)?.kind === 'runtime'
|
||||
|
||||
/** Repo ids an owner key can name, across both readings (see `ownerKeyWorktreeIds`). */
|
||||
function ownerKeyRepoIds(ownerKey: string | null | undefined): string[] {
|
||||
return ownerKey
|
||||
? ownerKeyWorktreeIds(ownerKey).flatMap((worktreeId) => {
|
||||
const repoId = splitWorktreeId(worktreeId)?.repoId
|
||||
return repoId ? [repoId] : []
|
||||
})
|
||||
: []
|
||||
}
|
||||
|
||||
/**
|
||||
* Repo ids that still own persisted rows but no longer appear in `state.repos`.
|
||||
*
|
||||
* Why nothing else finds them: every other sweeper is gated on the repo still being registered, so
|
||||
* deregistering a project stranded the rows it owned permanently — including a paired client's
|
||||
* mirror of a remote host's session partition, which no local repo removal can reach (#17776).
|
||||
* Rows owned by a `runtime:*` host are held live instead of swept: a paired client mirrors that
|
||||
* host's sessions without ever registering its repos, and this runs in the Store constructor,
|
||||
* before pairing, so catalog absence there proves nothing (#17776 read it as proof and deleted
|
||||
* live sessions). The cost is that residue outliving a removal is no longer swept for those hosts.
|
||||
*/
|
||||
export function collectDeregisteredRepoIds(state: PersistedState): Set<string> {
|
||||
const liveRepoIds = new Set(state.repos.map((repo) => repo.id))
|
||||
const retainOwner = (ownerKey: string | null | undefined): void => {
|
||||
for (const repoId of ownerKeyRepoIds(ownerKey)) {
|
||||
liveRepoIds.add(repoId)
|
||||
}
|
||||
}
|
||||
// `owners` goes unread: the walker only ever calls `addOwner`.
|
||||
const retainCollector = { owners: new Set<string>(), addOwner: retainOwner }
|
||||
for (const [hostId, session] of Object.entries(state.workspaceSessionsByHostId ?? {})) {
|
||||
if (session && isPairedHost(hostId)) {
|
||||
addWorkspaceSessionWorktreeOwners(session, retainCollector)
|
||||
}
|
||||
}
|
||||
for (const [worktreeId, meta] of Object.entries(state.worktreeMeta)) {
|
||||
if (isPairedHost(meta.hostId)) {
|
||||
retainOwner(worktreeId)
|
||||
}
|
||||
}
|
||||
for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) {
|
||||
if (isPairedHost(getExecutionHostIdFromWorktreeHostIdentity(alias))) {
|
||||
retainOwner(getWorktreeIdFromHostIdentity(alias))
|
||||
}
|
||||
}
|
||||
const orphanRepoIds = new Set<string>()
|
||||
// Only a full `<repoId>::<path>` locator seeds the set. A bare key -- a folder workspace id, a
|
||||
// repo-keyed topology revision, a test-shaped locator -- cannot be told apart from a repo id, and
|
||||
@@ -30,10 +72,7 @@ export function collectDeregisteredRepoIds(state: PersistedState): Set<string> {
|
||||
* other reading would hand the removal pass -- which accepts either -- a live row to delete.
|
||||
*/
|
||||
const addOwnerKey = (ownerKey: string): void => {
|
||||
const repoIds = ownerKeyWorktreeIds(ownerKey).flatMap((worktreeId) => {
|
||||
const repoId = splitWorktreeId(worktreeId)?.repoId
|
||||
return repoId ? [repoId] : []
|
||||
})
|
||||
const repoIds = ownerKeyRepoIds(ownerKey)
|
||||
if (repoIds.length > 0 && repoIds.every((repoId) => !liveRepoIds.has(repoId))) {
|
||||
for (const repoId of repoIds) {
|
||||
orphanRepoIds.add(repoId)
|
||||
|
||||
Reference in New Issue
Block a user