Skip to content

Commit 76cee97

Browse files
fix(runtime-host): isolate artifact invalidations from guests
1 parent 8bd7259 commit 76cee97

10 files changed

Lines changed: 41 additions & 39 deletions

‎apps/desktop/src/main/__tests__/main-startup-lifetime.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,17 @@ test('does not release renderer IPC before persistent handlers are registered',
124124
assert.doesNotMatch(readyBody, /runtimeHostManager\?\.start\(\)/u);
125125
});
126126

127+
test('wires Artifact deletion and purge events to preview invalidation for the active scope', () => {
128+
const subscribe = bootSource.indexOf('client.subscribeArtifactChanges(');
129+
const unsubscribe = bootSource.indexOf('unsubscribeArtifactChanges();', subscribe);
130+
assert.ok(subscribe >= 0 && unsubscribe > subscribe);
131+
const handler = bootSource.slice(subscribe, unsubscribe);
132+
assert.match(handler, /managedArtifactPreview\.revoke\(scope\.targetEpoch,\s*frame\.sessionId,\s*frame\.artifactId\)/u);
133+
assert.match(handler, /managedArtifactPreview\.releaseSession\(scope\.targetEpoch,\s*frame\.sessionId\)/u);
134+
assert.match(bootSource, /managedArtifactPreview\.openScope\(scope\.targetEpoch\)/u);
135+
assert.match(bootSource, /await managedArtifactPreview\.closeScope\(scope\.targetEpoch\)/u);
136+
});
137+
127138
test('resolves persisted locale before first post-settings recovery prompt', () => {
128139
const rendererRecoveryStart = earlyWindowSource.indexOf('onRendererProcessGone: async');
129140
const rendererRecovery = earlyWindowSource.slice(

‎apps/desktop/src/main/__tests__/managed-artifact-preview.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,15 +158,15 @@ test('bounds previews per session instead of starving another session', async ()
158158
}
159159
});
160160

161-
test('evicts the oldest lease at the global backstop without denying another session', async () => {
161+
test('does not invalidate existing previews when another session prepares one', async () => {
162162
const service = new ManagedArtifactPreview();
163163
try {
164164
const endpoints = [];
165165
for (let index = 0; index < 64; index += 1) {
166166
endpoints.push(await service.prepare('h', client(`preview-${index}`), `s${index}`, 'a1'));
167167
}
168168
const replacement = await service.prepare('h', client('replacement'), 's64', 'a1');
169-
await assert.rejects(fetch(endpoints[0]!.url));
169+
assert.equal(await (await fetch(endpoints[0]!.url)).text(), 'preview-0');
170170
assert.equal(await (await fetch(replacement.url)).text(), 'replacement');
171171
} finally {
172172
await service.close();

‎apps/desktop/src/main/managed-artifact-preview.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ import type { DesktopRuntimeHostClient } from './runtime-host-client.js';
2424

2525
export const PREVIEW_MAX_BYTES = 8 * 1024 * 1024;
2626
const MAX_PREVIEWS = 16;
27-
// Per-Session admission alone cannot bound the Desktop, so one global backstop
28-
// remains. It evicts the oldest lease instead of rejecting the newest: a global
29-
// rejection is what let one Session starve every other for a full TTL.
30-
const MAX_TOTAL_PREVIEWS = 64;
3127
const PREVIEW_TTL_MS = 30 * 60 * 1000;
3228
const READ_DEADLINE_MS = 30_000;
3329

@@ -82,10 +78,6 @@ export class ManagedArtifactPreview {
8278
if (sessionLeases.length >= MAX_PREVIEWS) {
8379
throw new Error('Too many active previews; wait for expiry');
8480
}
85-
if (this.leases.size >= MAX_TOTAL_PREVIEWS) {
86-
const oldest = this.leases.values().next().value as Lease | undefined;
87-
if (oldest) void this.release(oldest);
88-
}
8981
signal?.throwIfAborted();
9082
// Reserve before asynchronous reads, so concurrent preparations cannot exceed the bound.
9183
const lease: Lease = { scope, sessionId, artifactId, server: createServer() };

‎packages/runtime-host/src/__tests__/connection-session.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ test('concurrent responses remain framed and correlated in reverse completion or
137137
);
138138
});
139139

140-
test('scopes Session Guest Artifact changes to the shared Session', async () => {
140+
test('does not expose Artifact invalidations to Session Guests', async () => {
141141
const pair = await openTransportPair();
142142
const hostChanges = new HostChangeFeed();
143143
const session = new RuntimeHostConnectionSession({
@@ -184,11 +184,11 @@ test('scopes Session Guest Artifact changes to the shared Session', async () =>
184184
try {
185185
hostChanges.publishArtifactDeleted('session-2', 'artifact-2');
186186
hostChanges.publishArtifactDeleted('session-1', 'artifact-1');
187+
hostChanges.publishSessionCatalog('session-1');
187188
assert.deepEqual(decodeHostFrame(await pair.clientTransport.read(1_000)), {
188-
kind: 'artifact.changed',
189-
reason: 'deleted',
189+
kind: 'session.catalog.changed',
190+
revision: 1,
190191
sessionId: 'session-1',
191-
artifactId: 'artifact-1',
192192
});
193193
} finally {
194194
pair.clientTransport.abort();

‎packages/runtime-host/src/__tests__/host-change-feed.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,10 @@ test('routes each change kind only to subscribed connections', () => {
5656
);
5757
feed.attachConnection(
5858
'other-guest',
59-
{ sessionCatalog: { sessionId: 'session-1', principalId: 'guest-2' } },
59+
{
60+
artifact: { sessionId: 'session-1' },
61+
sessionCatalog: { sessionId: 'session-1', principalId: 'guest-2' },
62+
},
6063
{ send: async (frame) => void otherGuest.push(frame) },
6164
);
6265

‎packages/runtime-host/src/__tests__/session-retirement-coordinator.test.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ describe('Host Session retirement coordinator', () => {
196196
assert.equal(garbageBatches, 0);
197197
});
198198

199-
test('publishes Session Artifact invalidation only when Artifact purge succeeds', async () => {
199+
test('publishes Session Artifact invalidation even when a purge partially fails', async () => {
200200
const purged: string[] = [];
201201
await purgeSessionSidecars(
202202
{
@@ -225,7 +225,7 @@ describe('Host Session retirement coordinator', () => {
225225
),
226226
AggregateError,
227227
);
228-
assert.deepEqual(purged, ['session-success']);
228+
assert.deepEqual(purged, ['session-success', 'session-artifact-failure']);
229229

230230
await assert.rejects(
231231
purgeSessionSidecars(
@@ -243,7 +243,11 @@ describe('Host Session retirement coordinator', () => {
243243
),
244244
AggregateError,
245245
);
246-
assert.deepEqual(purged, ['session-success', 'session-other-failure']);
246+
assert.deepEqual(purged, [
247+
'session-success',
248+
'session-artifact-failure',
249+
'session-other-failure',
250+
]);
247251
});
248252

249253
test('rejects ordinary archive and remove operations for the Coordination Session', async () => {

‎packages/runtime-host/src/protocol/index.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,9 @@ export const RUNTIME_HOST_REGISTRATION_SCHEMA_VERSION = 1 as const;
106106
export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const;
107107
// Increment when the same protocol version no longer guarantees safe Client-Host
108108
// interoperability. Mismatches are rejected before domain commands are admitted.
109-
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 198 as const;
110-
// 198: Host change feed includes Artifact deletion and Session purge invalidation frames.
109+
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 199 as const;
110+
// 199: Artifact invalidation feed and guest routing require matching peers.
111+
// 198: Usage queries can filter by model call kind.
111112
// 197: Queue reorder requests carry the expected queue revision. Epoch-196
112113
// peers reject the required field or send an unfenced reorder request.
113114
// 196: `turn.start` carries an optional durable external-message origin; older

‎packages/runtime-host/src/server/connection-session.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -366,14 +366,14 @@ export class RuntimeHostConnectionSession {
366366
: hasRuntimeHostOperationGrant(this.#options.connection.authority, 'session.catalog.query')
367367
? true
368368
: undefined;
369+
// Artifact invalidations are consumed by the owner Desktop. Guest clients
370+
// must not learn Artifact identities or share a session-scoped subscription
371+
// whose lifetime is tied to another guest's catalog scope.
369372
const artifact: HostChangeSubscriptionMask['artifact'] =
370-
this.#options.connection.authority.principalKind === 'session_guest'
371-
? sharedSessionId !== undefined
372-
? { sessionId: sharedSessionId }
373-
: undefined
374-
: hasRuntimeHostOperationGrant(this.#options.connection.authority, 'artifact.query')
375-
? true
376-
: undefined;
373+
this.#options.connection.authority.principalKind !== 'session_guest' &&
374+
hasRuntimeHostOperationGrant(this.#options.connection.authority, 'artifact.query')
375+
? true
376+
: undefined;
377377
this.#hostChanges = service.attachConnection(
378378
this.#options.connection.connectionId,
379379
{

‎packages/runtime-host/src/server/host-change-feed.ts‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,6 @@ export class HostChangeFeed {
168168
this.#subscriptions.delete(connectionId);
169169
continue;
170170
}
171-
if (
172-
closeScopeFor !== undefined &&
173-
frame.kind === 'session.catalog.changed' &&
174-
subscription.mask.artifact !== true &&
175-
subscription.mask.artifact?.sessionId === frame.sessionId &&
176-
this.#subscriptions.get(connectionId) === subscription
177-
) {
178-
this.#subscriptions.delete(connectionId);
179-
}
180171
}
181172
}
182173
}

‎packages/runtime-host/src/server/session-sidecar-purge.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ export async function purgeSessionSidecars(
3939
...(authority.contextOffload ? [authority.contextOffload.retireSession(sessionId)] : []),
4040
authority.purgeOperationalState(sessionId),
4141
]);
42-
// Once Artifacts are purged their previews must stop serving, even when a
43-
// sibling sidecar fails and the aggregate error is thrown below.
44-
if (artifactPurge.status === 'fulfilled') authority.onArtifactsPurged?.(sessionId);
42+
// Invalidation is idempotent and must happen even when the Artifact purge
43+
// rejects: a failed purge may have removed only part of the Session's data.
44+
authority.onArtifactsPurged?.(sessionId);
4545
const failures = [artifactPurge, ...sidecarPurges].flatMap((outcome) =>
4646
outcome.status === 'rejected' ? [outcome.reason] : [],
4747
);

0 commit comments

Comments
 (0)