fix(desktop): invalidate artifact previews after deletion

[SummerC0zyR0ck]

Summary

Fixes #5341
Refs #5436

ManagedArtifactPreview (added in #5316) served each HTML Artifact from an in-memory snapshot behind a 30-minute TTL, and only two things ever released a lease: the Desktop artifacts:delete IPC caller and closeScope(targetEpoch).
Every other deletion path lived inside the Runtime Host and never reached the Desktop preview service:

• purgeSessionArtifacts during Session retirement and revision-discard (session-sidecar-purge),
• artifact.delete issued by any other Client, which carried no coupling to the preview layer.

So deleting a generated document did not stop it being readable: the lease kept serving the deleted bytes until its TTL expired.
Separately, the preview bound was a single global MAX_PREVIEWS = 16 with no eviction, so one Session could hold every lease and deny previews to every other Session for half an hour.
The fix makes deletion observable to the layer that owns the preview, and partitions the bound.

A Host-scoped Artifact invalidation frame.
A new closed artifact.changed Host frame carries either deleted (sessionId, artifactId) or session_purged (sessionId).
It is published by the layer that owns each deletion:

• HostArtifactCoordinator publishes deleted only after deleteUserArtifactInSession returns deleted — never for protected or not_found;
• purgeSessionSidecars publishes session_purged when the Artifact purge fulfilled, even when a sibling sidecar fails and the aggregate error is thrown below.

The frame is routed by the existing HostChangeFeed: clients holding artifact.query subscribe globally, and Session Guests are scoped to their shared Session (the same scoping mechanism as session.catalog.changed).
Because a new Host frame kind is a wire change, RUNTIME_HOST_COMPATIBILITY_EPOCH moves past the base (175 -> 176).

Desktop consumes the frame.
subscribeArtifactChanges is added to RuntimeHostConnection, the reconnecting connection, and DesktopRuntimeHostClient; runtime-host-boot calls ManagedArtifactPreview.revoke for deleted and the new releaseSession(scope, sessionId) for session_purged.
The existing direct revoke in the artifacts:delete caller is kept deliberately: stopping the bytes there must not depend on feed delivery to this Client.

Previews reopen after a Host reconnect. Refs #5436
A lost connection closes the candidate, whose teardown retires the target epoch in ManagedArtifactPreview; a reconnect reuses that same epoch, so prepare stayed permanently rejected with Preview owner is closed.
Registration now reopens the scope (ManagedArtifactPreview.openScope) before the replacement candidate serves requests, while the disconnect still releases the old endpoints.

The quota is per Session, with a global backstop.
Admission is bounded per (scope, sessionId) at 16; a single global backstop of 64 evicts the oldest lease instead of rejecting the newest, because a global rejection is exactly what let one Session starve every other.
This keeps a total memory bound without reintroducing the cross-Session denial.

Verification

Invalidation is published only after a deletion actually committed.

• A protected or missing Artifact never invalidates. artifact-coordinator.test.ts — "Artifact deletion publishes invalidation only after a committed delete".
• A Session purge invalidates on Artifact success, not on overall success. session-retirement-coordinator.test.ts — "publishes Session Artifact invalidation only when Artifact purge succeeds".

The frame is closed and correctly scoped.

• Closed wire shape. artifact-protocol.test.ts — "accepts closed Artifact change frames and rejects malformed invalidations".
• Routing by subscription. host-change-feed.test.ts — "routes each change kind only to subscribed connections".
• Session Guests see only their shared Session. connection-session.test.ts — "scopes Session Guest Artifact changes to the shared Session".
• A reconnect does not leak or replay. reconnecting-connection.test.ts — "a reconnecting Client forwards Artifact invalidations only from its current connection".
• Real transport end to end. artifact-two-client-uds.test.ts — "production Host ignores Artifact publication residue and preserves deletes across owner death".

The Desktop preview lifecycle.
managed-artifact-preview.test.ts: "bounds previews per session instead of starving another session"; "evicts the oldest lease at the global backstop without denying another session"; "releases every preview for a purged session"; and "delete and Session purge cancel previews that are still preparing".

Previews reopen after a reconnect (#5436).
runtime-host-desktop-candidate.test.ts — "closes old managed Artifact previews and reopens the scope after reconnect": the endpoint stops after the Host connection closes, and a replacement candidate under the same targetEpoch prepares and serves again.

Compatibility boundary.
protocol.test.ts — "publishes a new compatibility epoch for Artifact invalidation frames", alongside scripts/protocol-epoch-check.mjs (175 -> 176).

Checks run on the current head:

• npm run clean && npm run build — all workspaces.
• biome check; tsc --noEmit for @maka/runtime-host and the Desktop tsconfig.main.
• node scripts/protocol-epoch-check.mjs --base upstream/main --head HEAD — 175 -> 176.
• @maka/runtime-host affected suites (protocol, Artifact protocol, host change feed, connection session, Artifact coordinator, reconnecting connection, Session retirement) — pass.
• @maka/desktop managed-artifact-preview + runtime-host-desktop-candidate — pass.
• Earlier rebased heads also passed the full Desktop main suite and the CLI suite.

Changes since last review

• Rebased onto current main (e6db75689); the only conflict resolution is the compatibility-epoch bump to 176.
• Removed the Deep Research deletion publisher and its test: the workflow was retired upstream in refactor: retire the Deep Research workflow #5554 and HostDeepResearchCoordinator no longer exists.
• Dropped the earlier Desktop e2e queue-admission waits: upstream rewrote side-chat-followups.spec.ts, which supersedes them.
• The Artifact invalidation guarantee is unchanged; it now covers HostArtifactCoordinator deletes and Session purge.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex investigated the defect and fixed, authored the implementation review and Runtime/UI/TUI regression tests.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[me2seeks]

Reviewed exact head 136f20b15d83474d9f973fc267e3fcceaf8a4058. The deletion publishers, subscription scoping, and preview quotas are internally consistent, but the transient change-feed path still leaves deleted previews readable across a reconnect. One blocking P2 is recorded inline.

[me2seeks]

P2 — A deletion missed while reconnecting leaves the deleted preview live for the rest of its TTL. artifact.changed is a transient frame with no revision or replay, and RuntimeHostReconnectingConnection only rebinds this listener to the replacement connection. The existing preview scope is not closed when availability is lost. A reachable sequence is: Desktop prepares an HTML preview; its remote/SSH/WSL Host connection drops; the still-running Host deletes the Artifact through another Client, Deep Research rollback, or Session purge; the invalidation is emitted while no Desktop subscription exists; Desktop reconnects and receives only future frames. The local preview server therefore keeps serving the deleted snapshot for up to 30 minutes. The new reconnect test itself establishes the non-replay behavior by forwarding only frames emitted by the replacement connection, so the PR's deletion guarantee does not hold across a connection gap.

[SummerC0zyR0ck]

Thanks for the careful review, @me2seeks — the concern is fair, and it sent us back through the Desktop connection model in detail. Here is what we found, and where we would value your guidance.

On the Desktop, the reconnecting path you described does not appear to exist. RuntimeHostReconnectingConnection is constructed only by the CLI/TUI clients; no Desktop (main-process) code path builds one. So the "listener is rebound to the replacement connection while the old preview scope stays open" mechanism does not apply to the Desktop.

For the connections the Desktop does use:

• libp2p-direct peer: the Host reuses the same connection session across a resume — peer-listener.ts handles the resume branch and returns without calling accept again — so the change-feed subscription is never dropped, and outbound bytes are buffered and replayed (2 MiB window, 30 s recovery). We already have tests for both the replay (resumable-peer-stream: "one-way blackhole triggers automatic recovery and preserves the pending read", "real TCP replacement preserves one Host dispatcher…") and the session reuse (peer-listener: "…resume spends no slot").
• Non-resumable transports (WSL pipe, local transport, SSH/tls/plaintext websockets): a drop closes the RuntimeHostConnection; the candidate tears down and the existing teardown calls ManagedArtifactPreview.closeScope, releasing every lease for the scope. Covered by runtime-host-desktop-candidate: "tears down the whole candidate when the Host connection closes".
• If peer recovery exceeds 30 s or the send window, the stream closes and the connection closes too — the same closeScope path.

So we could not construct a Desktop sequence where a deletion is published while no subscription exists and the connection stays open. We removed the availability-based hook we had tried, because it only applies to reconnecting connections and would never fire here.

We may well be missing a path. If you have a specific one in mind — a transport, a mount, or a client we overlooked — we would be glad to hook the release to whatever signal actually fires there. Could you point us at it?

[me2seeks]

P1 — A normal Desktop reconnect permanently disables artifact previews for that target.

The concrete path is the candidate cleanup, not RuntimeHostReconnectingConnection: DesktopRuntimeHostCandidateImpl calls disposeClientIpc when connection.closed settles; registerHostClientIpc then calls managedArtifactPreview.closeScope(scope.targetEpoch) (runtime-host-boot.ts:1913). closeScope adds that epoch to retiredScopes (managed-artifact-preview.ts:74 rejects every retired scope). However, createDesktopRuntimeHostCandidate derives scope.targetEpoch from ipcMain.epoch (runtime-host-desktop-candidate.ts:549), and the Desktop manager creates every replacement candidate with the same target.epoch (runtime-host-desktop-manager.ts:1135). The replacement therefore reuses an already-retired scope.

I reproduced this on 9bd1819f6142d477726f8a9b5760aa5325df00f9: prepare('same-epoch') → closeScope('same-epoch') → prepare('same-epoch') returns Error: Preview owner is closed. After a normal WSL/SSH/local reconnect, existing preview leases are released but every later artifact preview for that target stays unavailable.

[SummerC0zyR0ck]

Thanks for tracing the concrete path — you are right. closeScope(targetEpoch) released the existing leases but also permanently retired an epoch that Desktop reuses across replacement candidates.

I fixed this by reopening the scope only after the replacement candidate successfully registers. Teardown still retires the scope and closes all old leases, so requests cannot create previews during the reconnect gap.

The regression test now verifies the complete lifecycle: the old preview URL becomes unreachable after disconnect, and a replacement candidate using the same targetEpoch can create and serve a new preview.

[SummerC0zyR0ck]

Hi @me2seeks — the P1 you found is fixed on the current head 6c444a5c.
Teardown still retires the scope and releases the old leases, and the replacement candidate reopens the scope only after it registers, so a normal reconnect can prepare previews again on the same target epoch.
The regression test now covers the full lifecycle: the old URL fails after the disconnect, and a replacement candidate on the same targetEpoch can prepare and serve a new preview.

Could you take another look at the current head when you have a moment? The required approval is the only thing left on my side.

[Astro-Han]

Reviewed exact head e1aa3d3792447de41b354ceaa22baa24ea9c7126 against base 672d82731a638e45e3ec87022eabdcf70a5a90be.

The production Artifact preview lifecycle fix remains internally consistent. apps/desktop/src/main/managed-artifact-preview.ts:59-61 reopens only transiently retired scopes, while the global close state remains terminal. apps/desktop/src/main/runtime-host-boot.ts:1911-1925 opens the target epoch after candidate owner registration and closes the scope before candidate teardown completes. Together with the same-epoch successor ordering in packages/runtime-host/src/client/reconnect-lifecycle.ts:300-346, a reconnecting candidate can prepare a preview again without allowing the old connection to keep forwarding Artifact events.

The new head only adds two synchronization assertions in apps/desktop/e2e/side-chat-followups.spec.ts:115-123,179-186. They wait for the queue's draggable handles before editing or injecting the disconnect gap; packages/ui/src/composer-message-queue.tsx:159-165,207-220 confirms that this selector represents queued, reorderable entries. The exact-head required test run 35183171316 / job 105079524815 passed, including affected tests, Runtime Host, Desktop E2E, Browser WebContentsView, WorkHub browser smoke, Alignment audit, and CLI release candidate validation.

I found no P0-P3 correctness issue in this exact head. Local build/typecheck/test and real Host/Electron reconnect smoke were not independently run because this worktree lacks the complete toolchain; the hosted run is the available execution evidence.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed exact head 8903dfd73dd3187c90c0f7d33a0d6c1bc23f518a against base 672d82731a638e45e3ec87022eabdcf70a5a90be.

Technical result: GO. I found no P0-P3 correctness, authorization, concurrency, or lifecycle issue in this exact head.

The production Artifact preview lifecycle is now coherent. apps/desktop/src/main/managed-artifact-preview.ts:59-61 removes only a transiently retired scope marker, while :78,184-202 continues to reject closed scopes and release every lease/server. apps/desktop/src/main/runtime-host-boot.ts:1640-1646 maps Host deleted frames to per-Artifact revoke and session_purged frames to per-Session release; :1911-1917 opens the target epoch after registration and closes it during candidate disposal. apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts:82-90 also keeps the direct-delete revoke independent of feed delivery.

The earlier same-epoch reconnect issue is fixed by ordering, not by leaving stale previews alive. apps/desktop/src/main/runtime-host-desktop-candidate.ts:313-364 makes candidate closure wait for client-IPC disposal, and packages/runtime-host/src/client/reconnect-lifecycle.ts:300-324,349-366 installs a successor only after the previous candidate is closed. The candidate test at apps/desktop/src/main/__tests__/runtime-host-desktop-candidate.test.ts:503-562 verifies old URL failure and successful preview preparation on a successor using the same target epoch.

I also checked the deletion publishers and protocol boundary: successful user deletion (packages/runtime-host/src/server/artifact-coordinator.ts:471-502), successful Deep Research deletion (deep-research-coordinator.ts:94-103), Session purge (session-sidecar-purge.ts:32-50), strict frame decoding (packages/runtime-host/src/protocol/artifact-change.ts:23-62), and permission/session routing (connection-session.ts:351-409, host-change-feed.ts:161-193). Desktop candidates use raw RuntimeHostConnection (apps/desktop/src/main/runtime-host-desktop-candidate.ts:367-409,467-523), not the CLI/TUI reconnecting wrapper. For the Desktop transports, libp2p-direct retains bounded unacknowledged writes during path recovery (packages/runtime-host/src/transport/resumable-peer-stream.ts:192-224,392-410,433-464); non-resumable connection loss closes the candidate and releases its scope.

The exact-head required test run 35192164606 / job 105106945341 passed, including build, typecheck, affected workspace tests, Runtime Host tests, Desktop E2E, Browser WebContentsView, WorkHub browser smoke, Alignment audit, and CLI release-candidate validation. The PR diff is 32 files (+845/-18); the last two commits only add queue-handle waits in apps/desktop/e2e/side-chat-followups.spec.ts:115-123,179-186.

Local build/typecheck/test and real Host/Electron reconnect smoke were not independently run because this worktree lacks complete node_modules, TypeScript, Vitest, and zod. The hosted exact-head run is the available execution evidence. This review is a technical assessment only and does not approve merging.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed exact head 6c444a5ca72fe6483e818784cff43cd2a775089a against base 0169d0731d476e70ad2afb6cd7fc97e5980120f8.

Technical result: GO. I found no P0-P3 correctness, authorization, concurrency, or lifecycle issue in this exact head.

The preview owner and invalidation paths are coherent. apps/desktop/src/main/managed-artifact-preview.ts:59-202 reopens transient scopes, bounds leases per Session and globally, checks ownership across asynchronous reads and HTTP readiness, and releases timers, servers, and listeners. apps/desktop/src/main/runtime-host-boot.ts:1641-1647,1912-1918 maps deleted to per-artifact revoke and session_purged to per-Session release, while direct deletion also revokes locally in runtime-host-artifacts-ipc-main.ts.

The earlier same-target-epoch reconnect issue is closed by lifecycle ordering. apps/desktop/src/main/runtime-host-desktop-candidate.ts:313-364 waits for IPC and connection cleanup, and packages/runtime-host/src/client/reconnect-lifecycle.ts:300-366 installs a successor only after the prior resource has closed. apps/desktop/src/main/__tests__/runtime-host-desktop-candidate.test.ts:503-562 verifies the old URL is unusable and a successor can prepare a new preview on the same target epoch.

I also checked successful deletion publishers and Session purge wiring (packages/runtime-host/src/server/artifact-coordinator.ts:471-502, deep-research-coordinator.ts:94-103, execution-composition.ts:936-945,1578-1586,2462-2501), strict frame decoding (packages/runtime-host/src/protocol/artifact-change.ts:23-61), and permission/session routing (connection-session.ts:351-409, host-change-feed.ts:161-193). The exact-head required test run 35300099814 / job 105460752806 passed, including build, typecheck, affected workspace tests, Runtime Host tests, Desktop E2E, Browser WebContentsView, WorkHub browser smoke, Alignment audit, and CLI release-candidate validation.

The PR diff is 32 files (+845/-18) against the merge-base. Local full build/typecheck/test and real Host/Electron reconnect smoke were not independently run because this worktree lacks the complete toolchain; the hosted exact-head run is the available execution evidence. This is a technical assessment only and does not approve merging.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

Exact-head review: caeced84bc1dd4d94be862686905f4961d475625

Outcome: code GO. I found no P0–P3 correctness, ownership, lifecycle, or concurrency finding in this exact head.

The 32-file change (+845/-18) was reviewed across Desktop Artifact preview invalidation, Runtime Host artifact-change protocol/feed/client forwarding, candidate teardown/reconnect, deletion/session-purge callbacks, and the regression tests. In particular, managed-artifact-preview.ts:59-202 owns scope/lease/revoke/release cleanup; runtime-host-boot.ts:1641-1647,1912-1927 consumes deletion/purge events and closes the target scope during candidate cleanup; and runtime-host-desktop-candidate.ts:313-364 plus runtime-host-desktop-candidate.test.ts:503-562 cover teardown completion followed by same-epoch preview reuse. The Host protocol/feed and current-connection listener replacement paths were also checked for stale-event and permission-routing issues.

The exact-head required test check is successful (run 35334657597, job 105566598847). The merge tree is clean and git diff --check passes. No schema or migration changes are included.

Limitations: the available worktree lacks the complete local dependency/toolchain set, so I did not independently run the full local build/typecheck/dist suite or a real Electron/Runtime Host reconnect smoke test. The detailed evidence report is available as reports/pr5394-caeced84-review.md in the review workspace.

This comment is an automated review and does not replace independent human review.

Automated review notice