Skip to content

Commit 456088c

Browse files
fix(runtime-host): align artifact preview review fixes
1 parent 96ee6a3 commit 456088c

4 files changed

Lines changed: 24 additions & 25 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ test('wires Artifact deletion and purge events to preview invalidation for the a
133133
assert.match(handler, /managedArtifactPreview\.releaseSession\(scope\.targetEpoch,\s*frame\.sessionId\)/u);
134134
assert.match(bootSource, /managedArtifactPreview\.openScope\(scope\.targetEpoch\)/u);
135135
assert.match(bootSource, /await managedArtifactPreview\.closeScope\(scope\.targetEpoch\)/u);
136+
assert.ok(
137+
bootSource.indexOf('await managedArtifactPreview.closeScope(scope.targetEpoch)') <
138+
bootSource.indexOf('unsubscribeArtifactChanges();'),
139+
);
136140
});
137141

138142
test('resolves persisted locale before first post-settings recovery prompt', () => {

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,9 @@ test('rejects previews above the global limit without invalidating existing sess
168168
await assert.rejects(service.prepare('h', client('replacement'), 's64', 'a1'), /across the Desktop/);
169169
assert.equal(await (await fetch(endpoints[0]!.url)).text(), 'preview-0');
170170
assert.equal(await (await fetch(endpoints[63]!.url)).text(), 'preview-63');
171+
await service.releaseUrl(endpoints[0]!.url);
172+
const replacement = await service.prepare('h', client('replacement'), 's64', 'a1');
173+
assert.equal(await (await fetch(replacement.url)).text(), 'replacement');
171174
} finally {
172175
await service.close();
173176
}
@@ -181,14 +184,24 @@ test('reserves aggregate preview bytes before streaming and releases reservation
181184
getArtifact: async () => ({ id: 'a1', sessionId: 's1', turnId: 't1', createdAt: 0, name: 'large.html', kind: 'html' as const, sizeBytes: PREVIEW_MAX_BYTES, source: 'tool_result' as const }),
182185
streamArtifact: async () => { await gate; throw new Error('test stream failure'); },
183186
};
187+
const successfulLarge = {
188+
getArtifact: large.getArtifact,
189+
streamArtifact: async (_sessionId: string, _artifactId: string, write: (chunk: Uint8Array) => Promise<void>) => {
190+
const bytes = Buffer.alloc(PREVIEW_MAX_BYTES);
191+
await write(bytes);
192+
return bytes.length;
193+
},
194+
};
184195
const pending = Array.from({ length: 16 }, (_, index) => service.prepare('h', large, `s${index + 1}`, 'a1'));
185196
try {
186197
// Each in-flight Artifact reserves 8 MiB, reaching the 128 MiB aggregate budget.
187198
await new Promise((resolve) => setImmediate(resolve));
188199
await assert.rejects(service.prepare('h', large, 's17', 'a1'), /memory limit/);
189200
resume();
190201
await Promise.all(pending.map((preparation) => assert.rejects(preparation, /test stream failure/)));
191-
assert.equal((await service.prepare('h', client(), 's1', 'a1')).reachable, true);
202+
const recovered = await service.prepare('h', successfulLarge, 's1', 'a1');
203+
assert.equal(recovered.reachable, true);
204+
await service.releaseUrl(recovered.url);
192205
} finally {
193206
resume();
194207
await Promise.allSettled(pending);

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

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ test('routes each change kind only to subscribed connections', () => {
5757
feed.attachConnection(
5858
'other-guest',
5959
{
60-
artifact: { sessionId: 'session-1' },
6160
sessionCatalog: { sessionId: 'session-1', principalId: 'guest-2' },
6261
},
6362
{ send: async (frame) => void otherGuest.push(frame) },
@@ -99,37 +98,24 @@ test('routes each change kind only to subscribed connections', () => {
9998
assert.equal(otherGuest.length, 3);
10099
});
101100

102-
test('routes Artifact invalidations by Session scope', () => {
101+
test('routes Artifact invalidations only to owner subscriptions', () => {
103102
const feed = new HostChangeFeed();
104103
const all: unknown[] = [];
105-
const scoped: unknown[] = [];
104+
const guest: unknown[] = [];
106105
feed.attachConnection(
107106
'all-artifacts',
108107
{ artifact: true },
109108
{
110109
send: async (frame) => void all.push(frame),
111110
},
112111
);
113-
feed.attachConnection(
114-
'session-artifacts',
115-
{ artifact: { sessionId: 'session-1' } },
116-
{
117-
send: async (frame) => void scoped.push(frame),
118-
},
119-
);
112+
feed.attachConnection('guest', {}, { send: async (frame) => void guest.push(frame) });
120113

121114
feed.publishArtifactDeleted('session-1', 'artifact-1');
122115
feed.publishArtifactSessionPurged('session-2');
123116

124117
assert.equal(all.length, 2);
125-
assert.deepEqual(scoped, [
126-
{
127-
kind: 'artifact.changed',
128-
reason: 'deleted',
129-
sessionId: 'session-1',
130-
artifactId: 'artifact-1',
131-
},
132-
]);
118+
assert.deepEqual(guest, []);
133119
});
134120

135121
test('keeps catalog revisions independent and removes failed subscriptions', async () => {

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export interface HostChangeSubscription {
4141
}
4242

4343
export interface HostChangeSubscriptionMask {
44-
readonly artifact?: true | { readonly sessionId: string };
44+
readonly artifact?: true;
4545
readonly configuration?: boolean;
4646
readonly connectionCatalog?: boolean;
4747
readonly projectCatalog?: boolean;
@@ -166,7 +166,6 @@ export class HostChangeFeed {
166166
this.#subscriptions.get(connectionId) === subscription
167167
) {
168168
this.#subscriptions.delete(connectionId);
169-
continue;
170169
}
171170
}
172171
}
@@ -175,10 +174,7 @@ export class HostChangeFeed {
175174
function isSubscribed(mask: HostChangeSubscriptionMask, frame: HostChangeFrame): boolean {
176175
switch (frame.kind) {
177176
case 'artifact.changed':
178-
return (
179-
mask.artifact === true ||
180-
(mask.artifact !== undefined && frame.sessionId === mask.artifact.sessionId)
181-
);
177+
return mask.artifact === true;
182178
case 'configuration.changed':
183179
return mask.configuration === true;
184180
case 'connection.catalog.changed':

0 commit comments

Comments
 (0)