feat(storage): move desktop SQLite ownership into a storage utility process
Main no longer opens desktop business SQLite. A single utility-process
storage owner holds the Assistant, Knowledge, attachment, document-result,
remote-binding and native model-ledger databases and runs whole domain
transactions; Main and Runtime callers use explicit async domain operations.
- Remove the Main readWithFallback path and the supervision worker's
separate business write connection; manifests are scanned read-only and
committed by the sole writer.
- Remote events are ACKed only after commit; process loss reports
unconfirmed writes instead of replaying them.
- Use native utilityProcess messaging with one in-flight bound (32) and
ordered waiting instead of capacity rejection and retries.
- Start the storage process before the window so startup overlaps.
- Keep short tool output in memory and store full output once in owned
storage; paging, branching, restart and deletion reuse that copy.
- Put required runtime launch files under goodbuddy-prefixed owned temp
directories and reclaim confirmed stale ones; stop writing duplicate
parsed.md.
- Add lint guards for SQLite ownership and architecture, concurrency
acceptance fixtures, and maintenance/architecture documentation.
Validation: typecheck and lint pass. Alternating baseline (edf019aa) vs
candidate runs show no regression on single-session, C01, C02, C04 and a
5-minute C06; Main p99 drops from ~21 ms to ~16.7 ms under concurrency.
Fresh portable copy (1.75M events) upgrade content unchanged; real-model
output paging passes with 4 external requests. Full npm test runs had
1-9 different timing-sensitive failures per run that pass when rerun.
Not covered: C03 review publication, C07 source body, app-level C05.
This commit is contained in:
@@ -13,6 +13,7 @@ import {
|
||||
} from 'node:fs/promises'
|
||||
import { join } from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import fs from 'node:fs/promises'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
agentPackageCatalogSchema,
|
||||
@@ -345,7 +346,7 @@ describe('AgentPackageManager remote install candidates', () => {
|
||||
await expect(lstat(unrelated)).resolves.toMatchObject({})
|
||||
})
|
||||
|
||||
it('downloads, verifies, publishes, and leases an exact install archive', async () => {
|
||||
it.each([false, true])('leases the old package until release and retries failed deletion (%s)', async failDeletion => {
|
||||
const archiveBytes = Buffer.alloc(3 * 64 * 1024 + 11, 0x5a)
|
||||
const fixture = await createFixture(
|
||||
'mirror',
|
||||
@@ -433,8 +434,15 @@ describe('AgentPackageManager remote install candidates', () => {
|
||||
cachedLease.release()
|
||||
cachedLease.release()
|
||||
await expect(readFile(lease.path)).resolves.toEqual(archiveBytes)
|
||||
const remove = vi.spyOn(fs, 'rm')
|
||||
if (failDeletion) remove.mockRejectedValueOnce(new Error('busy'))
|
||||
lease.release()
|
||||
lease.release()
|
||||
if (failDeletion) {
|
||||
await vi.waitFor(() => expect(remove).toHaveBeenCalled())
|
||||
await expect(readFile(lease.path)).resolves.toEqual(archiveBytes)
|
||||
await manager.getInventory()
|
||||
}
|
||||
await vi.waitFor(async () => {
|
||||
await expect(readFile(lease.path)).rejects.toBeDefined()
|
||||
})
|
||||
|
||||
@@ -5,7 +5,7 @@ import {
|
||||
verify
|
||||
} from 'node:crypto'
|
||||
import { constants } from 'node:fs'
|
||||
import {
|
||||
import fs, {
|
||||
lstat,
|
||||
mkdir,
|
||||
open,
|
||||
@@ -307,6 +307,9 @@ export class AgentPackageManager {
|
||||
async getInventory(
|
||||
options: { refresh?: boolean } = {}
|
||||
): Promise<AgentPackageInventory> {
|
||||
for (const directory of this.#pendingRemovals) {
|
||||
await this.#removeWhenUnused(directory).catch(() => undefined)
|
||||
}
|
||||
if (options.refresh) {
|
||||
this.#installed.clear()
|
||||
}
|
||||
@@ -1074,12 +1077,16 @@ export class AgentPackageManager {
|
||||
// Invalid cache entries remain visible as an invalid inventory state.
|
||||
}
|
||||
}
|
||||
return verified.sort((left, right) =>
|
||||
const [current, ...obsolete] = verified.sort((left, right) =>
|
||||
compareSemanticVersions(
|
||||
right.verified.descriptor.version,
|
||||
left.verified.descriptor.version
|
||||
)
|
||||
)[0]
|
||||
)
|
||||
for (const record of obsolete) {
|
||||
await this.#removeWhenUnused(record.directory).catch(() => undefined)
|
||||
}
|
||||
return current
|
||||
}
|
||||
|
||||
async #installArchive(
|
||||
@@ -1869,21 +1876,19 @@ export class AgentPackageManager {
|
||||
return
|
||||
}
|
||||
this.#leaseCounts.delete(key)
|
||||
if (this.#pendingRemovals.delete(key)) {
|
||||
void rm(key, {
|
||||
recursive: true,
|
||||
force: true
|
||||
}).catch(() => undefined)
|
||||
if (this.#pendingRemovals.has(key)) {
|
||||
void this.#removeWhenUnused(key).catch(() => undefined)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async #removeWhenUnused(directory: string): Promise<void> {
|
||||
this.#pendingRemovals.add(directory)
|
||||
if ((this.#leaseCounts.get(directory) ?? 0) > 0) {
|
||||
this.#pendingRemovals.add(directory)
|
||||
return
|
||||
}
|
||||
await rm(directory, { recursive: true, force: true })
|
||||
await fs.rm(directory, { recursive: true, force: true })
|
||||
this.#pendingRemovals.delete(directory)
|
||||
}
|
||||
|
||||
#runExclusive<T>(
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { join } from 'node:path'
|
||||
import { SqliteRuntimeSessionBindingStore } from '../agent/runtime-session-binding-store'
|
||||
import type { RuntimeSessionBindingStore } from '../agent/runtime-session-binding-store'
|
||||
import type { ResolvedModelProfile } from '../runtime-settings-store'
|
||||
import type {
|
||||
AgentRuntimeSelection,
|
||||
@@ -27,9 +26,8 @@ import {
|
||||
} from './managed-model-bridge'
|
||||
import { boundedDiagnostic } from './bounded-diagnostic'
|
||||
|
||||
const RUNTIME_BINDING_DATABASE_NAME = 'remote-runtime-bindings.sqlite'
|
||||
|
||||
export type ManagedRemoteExecutionServicesOptions = {
|
||||
bindingStore: RuntimeSessionBindingStore
|
||||
sshHostStore: SshHostStore
|
||||
agentServices: Pick<
|
||||
RemoteAgentServices,
|
||||
@@ -39,7 +37,6 @@ export type ManagedRemoteExecutionServicesOptions = {
|
||||
| 'connectionManager'
|
||||
| 'controllerState'
|
||||
>
|
||||
userDataPath: string
|
||||
appPath: string
|
||||
resourcesPath: string
|
||||
packaged: boolean
|
||||
@@ -60,7 +57,7 @@ export class ManagedRemoteExecutionServices {
|
||||
readonly runtimeInstallationManager: RemoteRuntimeInstallationManager
|
||||
readonly runtimeValidator: ManagedRemoteProjectRuntimeValidator
|
||||
readonly workspaceAccessFactory: ManagedRemoteWorkspaceAccessFactory
|
||||
readonly bindingStore: SqliteRuntimeSessionBindingStore
|
||||
readonly bindingStore: RuntimeSessionBindingStore
|
||||
|
||||
#disposePromise?: Promise<void>
|
||||
readonly #readiness: Promise<void>
|
||||
@@ -138,9 +135,7 @@ export class ManagedRemoteExecutionServices {
|
||||
installationManager:
|
||||
options.agentServices.installationManager
|
||||
})
|
||||
this.bindingStore = new SqliteRuntimeSessionBindingStore(
|
||||
join(options.userDataPath, RUNTIME_BINDING_DATABASE_NAME)
|
||||
)
|
||||
this.bindingStore = options.bindingStore
|
||||
this.#readiness = Promise.resolve()
|
||||
}
|
||||
|
||||
@@ -213,7 +208,6 @@ export class ManagedRemoteExecutionServices {
|
||||
}
|
||||
}
|
||||
await settle(() => this.runtimeInstallationManager.dispose())
|
||||
await settle(() => this.bindingStore.dispose())
|
||||
if (errors.length > 0) {
|
||||
throw new AggregateError(
|
||||
errors,
|
||||
|
||||
@@ -180,6 +180,17 @@ function harness(options: {
|
||||
}
|
||||
|
||||
describe('ManagedRemoteProjectRuntimeValidator', () => {
|
||||
it('invalidates a live lease when cancelled or when its Agent identity changes', async () => {
|
||||
const test = harness()
|
||||
const controller = new AbortController()
|
||||
const lease = await test.validator.validate({ ...test.input, signal: controller.signal })
|
||||
controller.abort(new Error('save cancelled'))
|
||||
expect(() => lease.assertCurrent()).toThrow('save cancelled')
|
||||
const current = await test.validator.validate(test.input)
|
||||
test.connection.status.draining = true
|
||||
expect(() => current.assertCurrent()).toThrow(/identity/iu)
|
||||
})
|
||||
|
||||
it('installs, refreshes, and leases the current Runtime', async () => {
|
||||
const test = harness()
|
||||
const lease = await test.validator.validate(test.input)
|
||||
|
||||
@@ -97,6 +97,7 @@ export class ManagedRemoteProjectRuntimeValidator
|
||||
let released = false
|
||||
return {
|
||||
assertCurrent: (): void => {
|
||||
input.signal.throwIfAborted()
|
||||
if (released) {
|
||||
throw new Error(
|
||||
'Remote Runtime validation lease is released'
|
||||
@@ -107,6 +108,7 @@ export class ManagedRemoteProjectRuntimeValidator
|
||||
'Remote Runtime validation connection is not ready'
|
||||
)
|
||||
}
|
||||
assertInputIdentity(input)
|
||||
let current: RemoteAgentConnection['capabilities']
|
||||
try {
|
||||
current = input.connection.capabilities
|
||||
|
||||
@@ -725,7 +725,7 @@ describe('RemoteEnvironmentPreparer', () => {
|
||||
expect(value.operationStore.remove).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not block a fresh prepare when stale staging cleanup fails', async () => {
|
||||
it('retains failed staging cleanup and retries it before overwriting the operation record', async () => {
|
||||
const originalTarget = target()
|
||||
let cleanupCalls = 0
|
||||
const value = fixture({
|
||||
@@ -757,10 +757,14 @@ describe('RemoteEnvironmentPreparer', () => {
|
||||
'remote-download',
|
||||
undefined,
|
||||
new AbortController().signal
|
||||
)).resolves.toBeUndefined()
|
||||
)).rejects.toThrow('Previous remote staging cleanup failed')
|
||||
|
||||
expect(value.prepare).not.toHaveBeenCalled()
|
||||
expect(value.operationStore.remove).not.toHaveBeenCalled()
|
||||
expect(value.operationStore.save).not.toHaveBeenCalled()
|
||||
await value.preparer.prepare(HOST_ID, 'remote-download', undefined, new AbortController().signal)
|
||||
expect(value.prepare).toHaveBeenCalledOnce()
|
||||
expect(value.cleanup).toHaveBeenCalledTimes(2)
|
||||
expect(value.cleanup).toHaveBeenCalledTimes(3)
|
||||
expect(value.operationStore.remove).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
|
||||
@@ -33,7 +33,6 @@ import type {
|
||||
VerifiedRemoteAgentInstallCandidate
|
||||
} from './agent-package-manager'
|
||||
import type {
|
||||
PendingRemoteEnvironmentOperation,
|
||||
RemoteEnvironmentOperationStore
|
||||
} from './remote-environment-operation-store'
|
||||
import type {
|
||||
@@ -548,13 +547,7 @@ export class RemoteEnvironmentPreparer implements Preparer {
|
||||
) => void,
|
||||
signal: AbortSignal
|
||||
): Promise<void> {
|
||||
let pending: PendingRemoteEnvironmentOperation | undefined
|
||||
try {
|
||||
pending = await this.#operationStore.load(hostId)
|
||||
} catch {
|
||||
await this.#operationStore.remove(hostId)
|
||||
return
|
||||
}
|
||||
const pending = await this.#operationStore.load(hostId)
|
||||
if (!pending) {
|
||||
return
|
||||
}
|
||||
@@ -570,19 +563,19 @@ export class RemoteEnvironmentPreparer implements Preparer {
|
||||
await this.#sshPool.acquireRemotePackageBootstrap(
|
||||
target,
|
||||
signal
|
||||
).catch(() => undefined)
|
||||
)
|
||||
try {
|
||||
if (!bootstrapLease) {
|
||||
return
|
||||
}
|
||||
assertLeaseMatchesTarget(bootstrapLease, target)
|
||||
await bootstrapLease.cleanup(
|
||||
const result = await bootstrapLease.cleanup(
|
||||
pending.operationId,
|
||||
{ signal }
|
||||
).catch(() => undefined)
|
||||
} finally {
|
||||
bootstrapLease?.release()
|
||||
)
|
||||
if (!result.cleaned && result.reason !== 'operation-unavailable') {
|
||||
throw new Error('Previous remote staging cleanup failed; retry environment preparation')
|
||||
}
|
||||
await this.#operationStore.remove(hostId)
|
||||
} finally {
|
||||
bootstrapLease.release()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { setImmediate as nextTurn } from 'node:timers/promises'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { AssistantProject } from '../../shared/assistant-contracts'
|
||||
import type {
|
||||
@@ -106,10 +107,11 @@ function target(): SshConnectionTarget {
|
||||
}
|
||||
|
||||
type HarnessOptions = {
|
||||
getProject?: () => AssistantProject
|
||||
getProject?: () => AssistantProject | Promise<AssistantProject>
|
||||
validateWorkspace?: (signal?: AbortSignal) => unknown | Promise<unknown>
|
||||
create?: () => AssistantProject
|
||||
update?: () => AssistantProject
|
||||
create?: () => AssistantProject | Promise<AssistantProject>
|
||||
update?: () => AssistantProject | Promise<AssistantProject>
|
||||
onSaving?: () => void
|
||||
resolveRuntimeSelection?: (
|
||||
layer: RuntimeSelectionLayer | undefined
|
||||
) => Promise<AgentRuntimeSelection>
|
||||
@@ -221,22 +223,26 @@ function harness(options: HarnessOptions = {}) {
|
||||
): AssistantProject => {
|
||||
calls.push(operation)
|
||||
writes.push(write)
|
||||
write.assertCurrent()
|
||||
structuredClone(write)
|
||||
return result
|
||||
}
|
||||
const createProject = vi.fn((write: (typeof writes)[number]) =>
|
||||
persist('database-create', write, options.create?.() ?? project())
|
||||
)
|
||||
const createProject = vi.fn(async (write: (typeof writes)[number]) => {
|
||||
await nextTurn()
|
||||
return persist('database-create', write, await (options.create?.() ?? project()))
|
||||
})
|
||||
const updateProject = vi.fn(
|
||||
(
|
||||
async (
|
||||
_id: string,
|
||||
_updatedAt: string,
|
||||
write: (typeof writes)[number]
|
||||
) => persist('database-update', write, options.update?.() ?? project())
|
||||
) => {
|
||||
await nextTurn()
|
||||
return persist('database-update', write, await (options.update?.() ?? project()))
|
||||
}
|
||||
)
|
||||
const service = new RemoteProjectSaveService({
|
||||
database: {
|
||||
getProject: vi.fn(() => options.getProject?.() ?? project()),
|
||||
getProject: vi.fn(async () => { await nextTurn(); return options.getProject?.() ?? project() }),
|
||||
createSshProject: createProject,
|
||||
updateSshProject: updateProject
|
||||
},
|
||||
@@ -270,7 +276,10 @@ function harness(options: HarnessOptions = {}) {
|
||||
return runtimeLease
|
||||
})
|
||||
},
|
||||
notify: (_owner, value) => progress.push(value.phase)
|
||||
notify: (_owner, value) => {
|
||||
progress.push(value.phase)
|
||||
if (value.phase === 'saving') options.onSaving?.()
|
||||
}
|
||||
})
|
||||
services.push(service)
|
||||
return {
|
||||
@@ -282,11 +291,52 @@ function harness(options: HarnessOptions = {}) {
|
||||
updateProject,
|
||||
request,
|
||||
connectionRelease,
|
||||
runtimeRelease
|
||||
runtimeRelease,
|
||||
runtimeLease,
|
||||
connection
|
||||
}
|
||||
}
|
||||
|
||||
describe('RemoteProjectSaveService', () => {
|
||||
it('keeps validation resources until a delayed write commits, including cancellation after dispatch', async () => {
|
||||
let commit!: (value: AssistantProject) => void
|
||||
const gate = new Promise<AssistantProject>(resolve => { commit = resolve })
|
||||
const value = harness({ create: () => gate })
|
||||
const owner = new Owner(10)
|
||||
const pending = value.service.save(owner, { intent: 'create', draft })
|
||||
await vi.waitFor(() => expect(value.createProject).toHaveBeenCalledOnce())
|
||||
value.service.cancelCurrent(owner)
|
||||
expect(value.runtimeRelease).not.toHaveBeenCalled()
|
||||
expect(value.connectionRelease).not.toHaveBeenCalled()
|
||||
commit(project())
|
||||
await expect(pending).resolves.toMatchObject({ id: projectId })
|
||||
expect(value.runtimeRelease).toHaveBeenCalledOnce()
|
||||
expect(value.connectionRelease).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('rejects invalidation after async preparation and before dispatch without serializing checks', async () => {
|
||||
const value = harness({ onSaving: () => {
|
||||
vi.mocked(value.runtimeLease.assertCurrent).mockImplementation(() => { throw new Error('validation stale') })
|
||||
} })
|
||||
await expect(value.service.save(new Owner(11), { intent: 'create', draft })).rejects.toThrow('validation stale')
|
||||
expect(value.createProject).not.toHaveBeenCalled()
|
||||
expect(value.runtimeRelease).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('waits for the update revision read before preparing or writing', async () => {
|
||||
let release!: (value: AssistantProject) => void
|
||||
const revision = new Promise<AssistantProject>(resolve => { release = resolve })
|
||||
const value = harness({ getProject: () => revision })
|
||||
const owner = new Owner(12)
|
||||
const pending = value.service.save(owner, { intent: 'update', draft: { ...draft, projectId } })
|
||||
await nextTurn()
|
||||
expect(value.calls).toEqual([])
|
||||
value.service.cancelCurrent(owner)
|
||||
release(project())
|
||||
await expect(pending).rejects.toMatchObject({ name: 'AbortError' })
|
||||
expect(value.updateProject).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('prepares the current Host and creates one stable project record', async () => {
|
||||
const value = harness()
|
||||
|
||||
@@ -319,9 +369,9 @@ describe('RemoteProjectSaveService', () => {
|
||||
'workspace/validate',
|
||||
'runtime',
|
||||
'runtime-current',
|
||||
'database-create',
|
||||
'host-current',
|
||||
'runtime-current',
|
||||
'database-create',
|
||||
'runtime-release',
|
||||
'workspace/close',
|
||||
'connection-release'
|
||||
|
||||
@@ -15,10 +15,8 @@ import type {
|
||||
AgentRuntimeSelection,
|
||||
RuntimeSelectionLayer
|
||||
} from '../../shared/runtime-selection-contracts'
|
||||
import type {
|
||||
AssistantDatabase,
|
||||
SshProjectWrite
|
||||
} from '../assistant/assistant-database'
|
||||
import type { SshProjectWrite } from '../assistant/assistant-database'
|
||||
import type { AssistantStoragePort, Awaitable } from '../assistant-storage-port'
|
||||
import type {
|
||||
SshConnectionTarget,
|
||||
SshHostStore
|
||||
@@ -115,12 +113,10 @@ type PreparedRemoteProject = Readonly<{
|
||||
}>
|
||||
|
||||
export type RemoteProjectSaveServiceOptions = {
|
||||
database: Pick<
|
||||
AssistantDatabase,
|
||||
| 'getProject'
|
||||
| 'createSshProject'
|
||||
| 'updateSshProject'
|
||||
>
|
||||
database: Pick<AssistantStoragePort, 'getProject'> & {
|
||||
createSshProject(write: Omit<SshProjectWrite, 'assertCurrent'>): Awaitable<AssistantProject>
|
||||
updateSshProject(projectId: string, expectedUpdatedAt: string, write: Omit<SshProjectWrite, 'assertCurrent'>): Awaitable<AssistantProject>
|
||||
}
|
||||
sshHosts: Pick<
|
||||
SshHostStore,
|
||||
'resolveConnectionTarget' | 'assertConnectionTargetCurrent'
|
||||
@@ -213,7 +209,7 @@ export class RemoteProjectSaveService {
|
||||
): Promise<AssistantProject> {
|
||||
const expectedUpdatedAt =
|
||||
request.intent === 'update'
|
||||
? this.#readUpdateRevision(request)
|
||||
? await this.#readUpdateRevision(request)
|
||||
: undefined
|
||||
const prepared = await this.#prepare(
|
||||
owner,
|
||||
@@ -224,6 +220,11 @@ export class RemoteProjectSaveService {
|
||||
this.#progress(owner, 'saving')
|
||||
active.controller.signal.throwIfAborted()
|
||||
const write = this.#projectWrite(request, prepared)
|
||||
this.#sshHosts.assertConnectionTargetCurrent(toCurrentSshConnectionTarget(prepared.target))
|
||||
assertConnectionIdentity(prepared.target, prepared.installation, prepared.connection)
|
||||
assertCriticalWorkspaceRead(prepared.connection)
|
||||
prepared.runtimeLease.assertCurrent()
|
||||
active.controller.signal.throwIfAborted()
|
||||
return request.intent === 'create'
|
||||
? this.#database.createSshProject(write)
|
||||
: this.#database.updateSshProject(
|
||||
@@ -332,10 +333,10 @@ export class RemoteProjectSaveService {
|
||||
}
|
||||
}
|
||||
|
||||
#readUpdateRevision(
|
||||
async #readUpdateRevision(
|
||||
request: Extract<RemoteProjectSaveRequest, { intent: 'update' }>
|
||||
): string {
|
||||
const project = this.#database.getProject(request.draft.projectId)
|
||||
): Promise<string> {
|
||||
const project = await this.#database.getProject(request.draft.projectId)
|
||||
if (
|
||||
project.kind !== 'user' ||
|
||||
project.channel !== undefined ||
|
||||
@@ -354,14 +355,11 @@ export class RemoteProjectSaveService {
|
||||
#projectWrite(
|
||||
request: RemoteProjectSaveRequest,
|
||||
prepared: PreparedRemoteProject
|
||||
): SshProjectWrite {
|
||||
): Omit<SshProjectWrite, 'assertCurrent'> {
|
||||
const {
|
||||
runtimeSelection,
|
||||
target,
|
||||
installation,
|
||||
connection,
|
||||
workspace,
|
||||
runtimeLease
|
||||
workspace
|
||||
} = prepared
|
||||
return {
|
||||
project: {
|
||||
@@ -374,14 +372,6 @@ export class RemoteProjectSaveService {
|
||||
kind: 'ssh',
|
||||
hostId: target.host.id,
|
||||
remoteRootPath: workspace.canonicalDisplayPath
|
||||
},
|
||||
assertCurrent: () => {
|
||||
this.#sshHosts.assertConnectionTargetCurrent(
|
||||
toCurrentSshConnectionTarget(target)
|
||||
)
|
||||
assertConnectionIdentity(target, installation, connection)
|
||||
assertCriticalWorkspaceRead(connection)
|
||||
runtimeLease.assertCurrent()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user