fix(persistence): folder, group and session-state store writes name their host or why they needn't (R4-2c) (#30797)

* fix(persistence): session, automation-label writes take the workspace's host; exempt minted-unique folder and group ids (R4-2c)

* fix(persistence): keep the session removal's spill rule; the twin guard was unreachable (review)
This commit is contained in:
Neil
2026-10-19 10:50:25 -07:00
committed by GitHub
parent 9ffb153dde
commit 236eba1ecf
17 changed files with 143 additions and 32 deletions
-7
View File
@@ -1,7 +1,6 @@
# Store writes whose first parameter is a bare repo or worktree id (R4-2), as Class.method.
# This is a RATCHET: entries may only be REMOVED. Take RepoHostTarget or HostWorktreeTarget.
# Prune after a fix: pnpm check:id-only-store-write-ratchet --prune
AutomationPersistence.snapshotAutomationRunWorkspaceDisplayName
MetadataLineageOperations.migrateWorktreeIdentity
MetadataLineageOperations.removeWorkspaceLineage
MetadataLineageOperations.removeWorktreeLineage
@@ -9,9 +8,3 @@ MetadataLineageOperations.removeWorktreeMeta
MetadataLineageOperations.setWorktreeLineage
MetadataLineageOperations.setWorktreeMeta
MetadataLineageOperations.setWorktreeMetaForHost
OrcadMigrationBlobInstallPersistence.installDeferredOrcadMigrationShellHistory
ProjectCollectionOperations.deleteProjectGroup
ProjectCollectionOperations.removeFolderWorkspace
ProjectCollectionOperations.updateFolderWorkspace
ProjectCollectionOperations.updateProjectGroup
SessionHostPartitionOperations.removeWorkspaceSessionStateForWorktree
@@ -32,13 +32,26 @@ export const NOT_HOST_TWINNED_PARAMS = new Map([
['SshProfileOperations.oldTargetId', 'the id names the host itself']
])
// The store keeps one row per id: minted UUIDs, de-duplicated on load, and a conflicting
// migration import is refused (orcad-catalog-row-identity.ts `selectNewRows`).
const UNIQUE_GROUP_ID = 'group ids are minted UUIDs, one row per id in the store'
const UNIQUE_FOLDER_ID = 'folder workspace ids are minted UUIDs, one row per id in the store'
/** Matches exempt by method, with the reason. Keep each reason true. */
export const NOT_HOST_TWINNED = new Map([
['AutomationPersistence.advanceAutomationNextRun', 'automation ids are minted UUIDs'],
['AutomationPersistence.deleteAutomation', 'automation ids are minted UUIDs'],
['AutomationPersistence.updateAutomation', 'automation ids are minted UUIDs'],
[
'OrcadMigrationBlobInstallPersistence.installDeferredOrcadMigrationShellHistory',
'a retry trigger: the id only gates a store-wide reinstall; nothing is written under it'
],
['ProfilePreferences.recordFeatureInteraction', 'a feature id, not a row'],
['ProjectCollectionOperations.deleteProjectGroup', UNIQUE_GROUP_ID],
['ProjectCollectionOperations.removeFolderWorkspace', UNIQUE_FOLDER_ID],
['ProjectCollectionOperations.updateFolderWorkspace', UNIQUE_FOLDER_ID],
['ProjectCollectionOperations.updateProject', 'one Project row spans every host'],
['ProjectCollectionOperations.updateProjectGroup', UNIQUE_GROUP_ID],
[
'RepoLifecycleOperations.reorderRepos',
'a whole-catalog permutation: a duplicated id fails its check, so it refuses instead of guessing'
+12 -2
View File
@@ -1,3 +1,5 @@
import { parseExecutionHostId } from '../../shared/execution-host'
import { hostWorktreeTarget } from '../../shared/worktree/host-qualified-identity'
import { ipcMain } from 'electron'
import type { Store } from '../persistence'
import type { AutomationService } from '../automations/service'
@@ -110,8 +112,16 @@ export function registerAutomationHandlers(store: Store, service: AutomationServ
)
ipcMain.handle(
'automations:snapshotWorkspaceName',
(_event, args: { workspaceId: string; displayName: string }): number =>
store.snapshotAutomationRunWorkspaceDisplayName(args.workspaceId, args.displayName)
(_event, args: { workspaceId: string; hostId?: string; displayName: string }): number => {
const host = parseExecutionHostId(args.hostId)
// No host, no label: a bare id would also relabel a same-id twin's runs.
return host
? store.snapshotAutomationRunWorkspaceDisplayName(
hostWorktreeTarget({ id: args.workspaceId, hostId: host.id }),
args.displayName
)
: 0
}
)
ipcMain.handle('automations:rendererReady', (): void => {
service.setRendererReady()
+31 -1
View File
@@ -12,6 +12,7 @@ import { join } from 'node:path'
import { tmpdir } from 'node:os'
import type { PersistedState } from '../shared/persisted-state-types'
import { toRuntimeExecutionHostId, toSshExecutionHostId } from '../shared/execution-host'
import { hostWorktreeTarget } from '../shared/worktree/host-qualified-identity'
// Stub the ~/.ssh/config parser so the SSH-import test drives the real Store with deterministic hosts, not the operator's actual ~/.ssh/config.
const { loadUserSshConfigMock, sshConfigHostsToTargetsMock } = vi.hoisted(() => ({
@@ -744,7 +745,10 @@ describe('Store', () => {
})
store.createAutomationRun(automation, new Date('2026-05-13T09:00:00Z').getTime())
const updatedCount = store.snapshotAutomationRunWorkspaceDisplayName('wt1', 'Deleted workspace')
const updatedCount = store.snapshotAutomationRunWorkspaceDisplayName(
hostWorktreeTarget({ id: 'wt1', hostId: 'local' }),
'Deleted workspace'
)
expect(updatedCount).toBe(1)
expect(store.listAutomationRuns(automation.id)[0].workspaceDisplayName).toBe(
@@ -752,6 +756,32 @@ describe('Store', () => {
)
})
it("deleting an SSH twin's workspace leaves the local twin's run label", async () => {
const store = await createStore()
store.addRepo(makeRepo())
const automation = store.createAutomation({
name: 'Nightly',
prompt: 'Run checks',
agentId: 'claude',
projectId: 'r1',
workspaceMode: 'existing',
workspaceId: 'wt1',
timezone: 'UTC',
rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0',
dtstart: new Date('2026-05-13T00:00:00Z').getTime()
})
store.createAutomationRun(automation, new Date('2026-05-13T09:00:00Z').getTime())
const before = store.listAutomationRuns(automation.id)[0].workspaceDisplayName
const updatedCount = store.snapshotAutomationRunWorkspaceDisplayName(
hostWorktreeTarget({ id: 'wt1', hostId: toSshExecutionHostId('ssh-1') }),
'SSH twin'
)
expect(updatedCount).toBe(0)
expect(store.listAutomationRuns(automation.id)[0].workspaceDisplayName).toBe(before)
})
it('persists automation run output snapshots across later status updates', async () => {
const store = await createStore()
store.addRepo(makeRepo())
@@ -1,3 +1,5 @@
import { toSshExecutionHostId } from '../shared/execution-host'
import { hostWorktreeTarget } from '../shared/worktree/host-qualified-identity'
import {
closeTestStores,
testState,
@@ -421,7 +423,9 @@ describe('Store host-partitioned workspace sessions', () => {
store.setWorkspaceSession(session, 'runtime:env-a')
store.setWorkspaceSession(session, 'runtime:env-b')
store.removeWorkspaceSessionStateForWorktree(worktreeId, 'runtime:env-a')
store.removeWorkspaceSessionStateForWorktree(
hostWorktreeTarget({ id: worktreeId, hostId: 'runtime:env-a' })
)
store.setWorkspaceSession(session, 'runtime:env-a')
store.flush()
@@ -622,6 +626,25 @@ describe('Store host-partitioned workspace sessions', () => {
expect(store.getWorkspaceSession('local').tabsByWorktree[worktreeId]).toBeUndefined()
})
it("removing one SSH host's copy leaves a same-id copy on another SSH host", async () => {
const store = await createStore()
const worktreeId = 'repo-twin::/workspace/shared'
const session = {
...makeHostSession('repo-twin'),
tabsByWorktree: { [worktreeId]: [makeTerminalTab({ id: 'twin-tab', worktreeId })] }
}
store.setWorkspaceSession(session, toSshExecutionHostId('ssh-a'))
store.setWorkspaceSession(session, toSshExecutionHostId('ssh-b'))
store.removeWorkspaceSessionStateForWorktree(
hostWorktreeTarget({ id: worktreeId, hostId: toSshExecutionHostId('ssh-a') })
)
const tabsOn = (host: string) => store.getWorkspaceSession(host).tabsByWorktree[worktreeId]
expect(tabsOn(toSshExecutionHostId('ssh-a'))).toBeUndefined()
expect(tabsOn(toSshExecutionHostId('ssh-b'))?.[0]?.id).toBe('twin-tab')
})
// Trade-off: the fence is repo-wide, so claiming authority over a partition main never wrote would
// rebase every sibling worktree of that repo onto an empty copy. A delayed write wins here instead.
it('does not fence a delayed session write when its host partition was never persisted', async () => {
@@ -634,7 +657,9 @@ describe('Store host-partitioned workspace sessions', () => {
}
}
store.removeWorkspaceSessionStateForWorktree(worktreeId, 'runtime:env-a')
store.removeWorkspaceSessionStateForWorktree(
hostWorktreeTarget({ id: worktreeId, hostId: 'runtime:env-a' })
)
store.setWorkspaceSession(delayedSession, 'runtime:env-a')
const session = store.getWorkspaceSession('runtime:env-a')
@@ -1,3 +1,4 @@
import type { HostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import type {
Automation,
AutomationCreateInput,
@@ -188,10 +189,13 @@ export class AutomationPersistence {
return updateAutomationRunOperation(getAutomationRunOperations(this), result)
}
snapshotAutomationRunWorkspaceDisplayName(workspaceId: string, displayName: string): number {
snapshotAutomationRunWorkspaceDisplayName(
target: HostWorktreeTarget,
displayName: string
): number {
return snapshotAutomationRunWorkspaceDisplayNameOperation(
getAutomationRunOperations(this),
workspaceId,
target,
displayName
)
}
@@ -7,6 +7,7 @@ import {
normalizeExecutionHostId,
type ExecutionHostId
} from '../../../shared/execution-host'
import type { HostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import { getDefaultWorkspaceSession } from '../../../shared/constants'
import { pruneLocalTerminalScrollbackBuffers } from '../../../shared/workspace-session-terminal-buffers'
import { pruneWorkspaceSessionBrowserHistory } from '../../../shared/workspace-session-browser-history'
@@ -112,12 +113,11 @@ export class SessionHostPartitionOperations {
}
removeWorkspaceSessionStateForWorktree(
worktreeId: string,
hostId?: ExecutionHostId | null,
target: HostWorktreeTarget,
options: { advanceTerminalTopologyRevision?: boolean } = {}
): void {
for (const resolved of workspaceSessionPartitionIdsForHost(hostId)) {
removeWorkspaceSessionOwnerInPartition(this, worktreeId, resolved, options)
for (const resolved of workspaceSessionPartitionIdsForHost(target.hostId)) {
removeWorkspaceSessionOwnerInPartition(this, target.id, resolved, options)
}
}
}
@@ -1,3 +1,4 @@
import { hostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import { closeTestStores, createSqliteTestStore } from '../../persistence-test-harness'
/**
* Drives the real `Store`, because the value of the prune gate is entirely in whether the shipping
@@ -78,7 +79,9 @@ describe('store signals to the worktree metadata prune gate', () => {
} as unknown as WorkspaceSessionState)
parkGate()
store.removeWorkspaceSessionStateForWorktree(WORKTREE_ID)
store.removeWorkspaceSessionStateForWorktree(
hostWorktreeTarget({ id: WORKTREE_ID, hostId: 'local' })
)
expect(isLocalWorktreeMetadataPruneDue(REPO_ID)).toBe(true)
})
@@ -1,3 +1,4 @@
import { hostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import { closeTestStores, createSqliteTestStore } from '../../persistence-test-harness'
/**
* Drives the real `Store`, not the helper and not a fake.
@@ -155,7 +156,7 @@ describe('Store keeps runtime-authored session fields across desktop writes', ()
}
})
store.removeWorkspaceSessionStateForWorktree(WT)
store.removeWorkspaceSessionStateForWorktree(hostWorktreeTarget({ id: WT, hostId: 'local' }))
expect(
Object.keys(store.getWorkspaceSession().clientHostedBrowserPagesByWorktree ?? {})
@@ -1,3 +1,4 @@
import { hostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import { describe, expect, it, vi } from 'vitest'
import type { ExecutionHostId } from '../../../shared/execution-host'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
@@ -100,7 +101,9 @@ export function registerWorkspaceSessionWriteListenersTests(): void {
.catch((error: unknown) => error instanceof ProfileStateWriterError || error)
})
await step(() => store.retirePtyBinding(binding))
await step(() => store.removeWorkspaceSessionStateForWorktree(WT))
await step(() =>
store.removeWorkspaceSessionStateForWorktree(hostWorktreeTarget({ id: WT, hostId: 'local' }))
)
return {
saved: withStableFixtureIds({
captures: authority.captures,
@@ -1,3 +1,4 @@
import { hostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
@@ -160,8 +161,12 @@ describe('WSL move session freeze: other session writers', () => {
})
it('keeps the frozen rows when the workspace owner is removed', () => {
store.removeWorkspaceSessionStateForWorktree(MOVING)
store.removeWorkspaceSessionStateForWorktree(STAYING)
store.removeWorkspaceSessionStateForWorktree(
hostWorktreeTarget({ id: MOVING, hostId: 'local' })
)
store.removeWorkspaceSessionStateForWorktree(
hostWorktreeTarget({ id: STAYING, hostId: 'local' })
)
expect(titles(store)).toEqual({ moving: ['slept'], staying: undefined })
})
})
@@ -12,6 +12,10 @@ import {
collectPersistedWorktreeIdsForSessionHydration
} from '../../../shared/worktree/degraded-repo-worktree-validity'
import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id'
import {
hostWorktreeTarget,
type HostWorktreeTarget
} from '../../../shared/worktree/host-qualified-identity'
import type { DetectedWorktreeListResult } from '../../../shared/worktree/types'
import { getLocalWorktreeCatalogVersion } from '../../local-worktree-scan-generation'
@@ -21,7 +25,7 @@ type PurgeStore = {
getRepos(): readonly { id: string }[]
getWorkspaceSession(hostId?: string | null): WorkspaceSessionState
getWorkspaceSessionHostIds(): ExecutionHostId[]
removeWorkspaceSessionStateForWorktree(worktreeId: string, hostId?: ExecutionHostId | null): void
removeWorkspaceSessionStateForWorktree(target: HostWorktreeTarget): void
}
const ownedByPairedHost = (hostId: string | null | undefined): boolean =>
@@ -54,7 +58,7 @@ function dropInvalidWorkspaces(
continue
}
for (const key of invalidWorkspaceKeys(store.getWorkspaceSession(hostId), catalog, inScope)) {
store.removeWorkspaceSessionStateForWorktree(key, hostId)
store.removeWorkspaceSessionStateForWorktree(hostWorktreeTarget({ id: key, hostId }))
}
}
}
@@ -17,6 +17,8 @@ import {
compareAutomationRunsNewestFirst,
paginateAutomationRuns
} from '../../../shared/automation-run-cursor'
import type { HostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity'
import { parseWorkspaceKey } from '../../../shared/workspace-scope'
import {
normalizeAutomationPrecheckResult,
normalizeAutomationRunOutputSnapshot,
@@ -218,16 +220,23 @@ export function updateAutomationRun(
export function snapshotAutomationRunWorkspaceDisplayName(
operations: AutomationRunOperations,
workspaceId: string,
target: HostWorktreeTarget,
displayName: string
): number {
const normalizedDisplayName = normalizeAutomationRunWorkspaceDisplayName(displayName)
if (!normalizedDisplayName) {
return 0
}
// Folder ids are minted UUIDs (one row per id), and a run's project host may differ from it.
const hostScoped = parseWorkspaceKey(target.id)?.type !== 'folder'
let updatedCount = 0
operations.state.automationRuns = (operations.state.automationRuns ?? []).map((run) => {
if (run.workspaceId !== workspaceId || run.workspaceDisplayName === normalizedDisplayName) {
if (
run.workspaceId !== target.id ||
// A same-id twin's run names its own host; a run from before host context has none.
(hostScoped && run.runContext && run.runContext.hostId !== target.hostId) ||
run.workspaceDisplayName === normalizedDisplayName
) {
return run
}
updatedCount += 1
@@ -1,3 +1,4 @@
import { hostWorktreeTarget } from '../shared/worktree/host-qualified-identity'
import { closeTestStores, createSqliteTestStore, testState } from './persistence-test-harness'
import { Store as CanonicalStore } from './persistence/loading-store/store'
import { initDataPath as initCanonicalDataPath } from './persistence/loading-store/user-data-path'
@@ -109,7 +110,7 @@ describe('worktree removal across host session partitions', () => {
it('does not materialize an unwritten host partition when removing session state directly', async () => {
const store = await createStore()
store.removeWorkspaceSessionStateForWorktree(STALE, ENV_A)
store.removeWorkspaceSessionStateForWorktree(hostWorktreeTarget({ id: STALE, hostId: ENV_A }))
expect(store.getWorkspaceSessionHostIds()).toEqual(['local'])
})
+6 -1
View File
@@ -1,3 +1,4 @@
import type { ExecutionHostId } from '../../shared/execution-host'
import type {
AutomationDispatchRequest,
AutomationDispatchResult,
@@ -35,7 +36,11 @@ export type AutomationsApi = {
runId: string
}) => Promise<AutomationPrecheckResult | null>
markDispatchResult: (result: AutomationDispatchResult) => Promise<AutomationRun>
snapshotWorkspaceName: (args: { workspaceId: string; displayName: string }) => Promise<number>
snapshotWorkspaceName: (args: {
workspaceId: string
hostId: ExecutionHostId
displayName: string
}) => Promise<number>
rendererReady: () => Promise<void>
onDispatchRequested: (callback: (request: AutomationDispatchRequest) => void) => () => void
onChanged: (callback: (payload: AutomationsChangedPayload) => void) => () => void
+6 -2
View File
@@ -1,3 +1,4 @@
import type { ExecutionHostId } from '../../shared/execution-host'
import { ipcRenderer } from 'electron'
import type { ExternalAutomationManagerResult, PreloadApi } from '../api-types'
import type {
@@ -29,8 +30,11 @@ export const automationsApi = {
ipcRenderer.invoke('automations:runPrecheck', args),
markDispatchResult: (result: AutomationDispatchResult): Promise<AutomationRun> =>
ipcRenderer.invoke('automations:markDispatchResult', result),
snapshotWorkspaceName: (args: { workspaceId: string; displayName: string }): Promise<number> =>
ipcRenderer.invoke('automations:snapshotWorkspaceName', args),
snapshotWorkspaceName: (args: {
workspaceId: string
hostId: ExecutionHostId
displayName: string
}): Promise<number> => ipcRenderer.invoke('automations:snapshotWorkspaceName', args),
rendererReady: (): Promise<void> => ipcRenderer.invoke('automations:rendererReady'),
onDispatchRequested: (callback: (request: AutomationDispatchRequest) => void): (() => void) => {
const listener = (_event: Electron.IpcRendererEvent, request: AutomationDispatchRequest) =>
@@ -225,10 +225,11 @@ export function createRemoveWorktree(
}
const worktreeDisplayName = worktreeBeforeRemoval?.displayName?.trim()
if (worktreeDisplayName) {
if (worktreeDisplayName && hostId) {
try {
await window.api.automations?.snapshotWorkspaceName?.({
workspaceId: worktreeId,
hostId,
displayName: worktreeDisplayName
})
} catch (error) {