Skip to content

fix(desktop): managed artifact previews outlive deletion and share one global quota #5341

Description

@jackwener

Two lifecycle defects in the managed HTML artifact preview landed with #5316 (merged as 512605fd10f1cef0a9f65c1605c15eb5e2cf31fb). Both were reported as P2 on the PR before it merged — by the Qronos review line, and independently confirmed against the source by me — so this issue exists to give them a schedulable home rather than leaving them in review comments on a merged PR.

The endpoint's isolation properties are not in question here: loopback-only binding, the 256-bit bearer path, exact Host and whole-string URL matching, GET/HEAD only and the CSP all hold. These are lifecycle problems.

1. A deleted artifact stays served until its TTL expires

runtime-host-artifacts-ipc-main.ts calls preview.service.revoke(...) after deleteArtifact, and that is the only artifact deletion in apps/desktop/src. Deletion can originate elsewhere, and none of those paths notify ManagedArtifactPreview:

  • packages/runtime-host/src/server/deep-research-coordinator.ts:91 — an agent deleting its own artifact via deleteOwnedArtifactInSession.
  • packages/runtime-host/src/server/session-sidecar-purge.ts:36 and session-retirement-coordinator.ts:125 — purgeSessionArtifacts(sessionId) during session teardown.
  • packages/runtime-host/src/server/artifact-coordinator.ts:70 — the artifact.delete operation itself carries no coupling to the preview service; the revoke lives one layer up, in the Desktop caller.

closeScope does not cover this either: it is keyed on targetEpoch, not sessionId, so purging a session's artifacts leaves that session's leases running. The lease then serves the deleted artifact's bytes for the remainder of its 30-minute TTL.

The disclosure is bounded — the bearer path was already handed to the browser that opened the preview, so no new party gains access. The problem is that deleting a generated document is exactly the action a user takes to stop it being readable.

2. The preview quota is global, so one session can starve every other

apps/desktop/src/main/managed-artifact-preview.ts:71 tests this.leases.size >= MAX_PREVIEWS with no partition by scope or session. With MAX_PREVIEWS = 16, a 30-minute TTL, no eviction, and ArtifactPreview published as a model-callable tool, one session can hold every lease and deny previews to all other sessions for half an hour. The thrown message — "Too many active previews; wait for expiry" — describes the situation accurately: there is no recourse short of waiting.

Suggested directions

For (1), the revoke belongs at the layer that owns deletion rather than at one caller of it, so every path reaches it; alternatively the preview service could key leases by sessionId so session teardown releases them. For (2), partitioning the bound per session, or evicting the oldest lease instead of refusing the newest, would both remove the cross-session denial.


Filed by an automated agent (@kabi-opus) through a shared account, at the request of the review orchestration line. The two defects were first raised by the Qronos review line on #5316; the file and line references above are my own tracing of the source. This does not substitute for independent human review.

Activity

  1. SummerC0zyR0ck commented on Sep 15, 2026

    @SummerC0zyR0ck
    Contributor

    I'd like to take this one — the two managed-preview lifecycle defects, not the endpoint isolation properties.

    Plan:

    • Make artifact removal a Host-owned projection covering artifact.delete, deleteOwnedArtifactInSession and purgeSessionArtifacts, and subscribe to it where the preview service lives, so the revoke no longer hangs off the single artifacts:delete IPC caller.
    • Carry the session through lease identity: add a session-filtered release wired to session teardown and observation close, leaving closeScope to host-epoch retirement.
    • Replace the single global admission test with a per-session bound and keep MAX_PREVIEWS as a backstop, and/or evict the oldest lease instead of refusing the newest.
    • Focused coverage: cross-session admission, a Host-originated delete and a session purge each releasing the lease before its TTL, unchanged TTL/expiry behaviour, and the loopback binding, bearer path, exact Host/URL matching, GET/HEAD-only and CSP invariants untouched.
    中文摘要

    我想认领这个 issue,范围是托管预览的两个生命周期缺陷,不涉及端点隔离属性。

    计划:

    • 把产物删除做成 Host 拥有的投影,覆盖 artifact.delete、deleteOwnedArtifactInSession、purgeSessionArtifacts,由持有预览服务的组件订阅,这样 revoke 不再只挂在 artifacts:delete 这一个 IPC 调用点上。
    • 把 session 纳入 lease 身份:增加按会话过滤的释放,挂到会话清理和观察关闭上;closeScope 继续只负责 host-epoch 退役。
    • 把单一全局准入判断改为按会话限额,MAX_PREVIEWS 作为兜底;或改为淘汰最旧 lease 而不是拒绝最新。
    • 覆盖测试:跨会话准入、Host 侧删除与会话清理都能在 TTL 前释放 lease、TTL/过期行为不变,以及回环绑定、bearer 路径、严格 Host/URL 匹配、仅 GET/HEAD 与 CSP 这些不变量保持不变。
  2. SummerC0zyR0ck commented on Sep 15, 2026

    @SummerC0zyR0ck
    Contributor

    take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions