Skip to content

GC unreferenced declared screenshot captures via the manifest - #6055

Open
lukemelia wants to merge 1 commit into
mainfrom
cs-12728-declared-screenshot-ledger-rows-leak-when-a-slot-is-renamed
Open

GC unreferenced declared screenshot captures via the manifest#6055
lukemelia wants to merge 1 commit into
mainfrom
cs-12728-declared-screenshot-ledger-rows-leak-when-a-slot-is-renamed

Conversation

@lukemelia

Copy link
Copy Markdown
Contributor

What this does

Closes the declared-lane leak in the MediaCache GC (CS-12728): a slot that is renamed, deleted, or re-specced mints a new specHash, so its old ledger row was never superseded (no newer row shares its identity), never tombstoned (the source lives), and had no expiry arm — the row and its object leaked permanently.

findMediaCacheGcCandidates gains an unreferenced arm: a declared-lane row whose spec hash no live prerendered_html row's screenshots manifest names is collectible. The manifest is the right liveness set:

  • ?name= serving is pinned to the manifest, so a row it doesn't reference is already unreachable by any serve;
  • carry-forwards (keyBy: 'file-content') keep their spec hash in the manifest across generations — which is exactly what protects their older-generation rows from this arm;
  • error renders preserve the last-known-good manifest, so a failing source keeps protecting its captures.

Two guards keep the arm conservative:

  1. A live prerendered row must exist. A source mid-first-index has no row yet; absence of evidence must not read as an empty roster.
  2. Manifest stability: every live row's rendered_at must be older than GC min-age, so a manifest published moments ago — an author mid-iteration, a transient capture failure the retry lane is still working — collects nothing until the state has held for a full window.

Realm-copied rows answer to their own realm's manifest (the copy duplicates both together, keeping them consistent). The predicate lives in one helper used by both the reason CASE and the WHERE, mirroring the tombstone arm's no-drift pattern. The sweep's SQL is Postgres-only as before; this adds jsonb_each over the manifest, with a jsonb_typeof guard so a malformed manifest can't error the whole sweep.

Test plan

Seven new cases in the GC suite (real Postgres, direct row seeding): re-specced slot reclaimed while the referenced one survives; manifest-less live row reclaims all declared captures; a freshly published manifest collects nothing until min-age; carry-forward protection across generations; no-prerendered-row conservatism; per-realm manifest scoping for realm copies; and the on-demand lane's immunity to the arm. All 15 pre-existing GC/putMedia tests pass unchanged.

🤖 Generated with Claude Code

A declared-lane ledger row was only reclaimable when superseded (a
newer generation under the same capture identity) or when its source
was tombstoned. A slot that is renamed, deleted, or re-specced mints a
new specHash, so its old row met neither arm and leaked its object
permanently — and authors iterating on declarations mint these
constantly.

The sweep gains an 'unreferenced' arm: a declared-lane row whose spec
hash no live prerendered_html row's screenshots manifest names is
collectible. The manifest is the correct liveness set — ?name= serving
is pinned to it, carry-forwards keep their hash in it across
generations, and error renders preserve the last-known-good manifest —
so a row it doesn't reference is unreachable by any serve. Two guards
keep the arm conservative: a live prerendered row must exist (a source
mid-first-index has none, and absence must not read as an empty
roster), and every live row's rendered_at must be older than min-age,
so a manifest published moments ago (mid-iteration churn, a capture
failure the retry lane is still working) collects nothing until the
state has held for a full window. Realm-copied rows answer to their own
realm's manifest, which the copy keeps consistent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Host Test Results

    1 files      1 suites   2h 38m 16s ⏱️
4 677 tests 4 663 ✅ 14 💤 0 ❌
4 692 runs  4 678 ✅ 14 💤 0 ❌

Results for commit 2fa00c6.

Realm Server Test Results

    1 files    203 suites   1h 10m 17s ⏱️
2 662 tests 2 662 ✅ 0 💤 0 ❌
2 701 runs  2 701 ✅ 0 💤 0 ❌

Results for commit 2fa00c6.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The new GC arm is implemented conservatively, is factored to avoid CASE/WHERE drift, and is covered by focused new Postgres-backed tests that validate the key edge cases described in the PR.

Pull request overview

This PR updates the MediaCache GC logic to reclaim “declared” screenshot ledger rows that are no longer reachable because their capture spec hash is no longer referenced by any live prerendered_html.screenshots manifest, closing a long-lived leak when slots are renamed/deleted/re-specced.

Changes:

  • Add a new GC reason ('unreferenced') and a Postgres-only SQL predicate that treats the screenshots manifest as the liveness set for declared captures, with conservatism guards (requires a live prerendered row; requires manifest stability past min-age).
  • Extend the GC test suite with new Postgres-seeded scenarios covering unreferenced reclamation, stability guard behavior, carry-forward protection, realm scoping, and on-demand immunity.
File summaries
File Description
packages/runtime-common/tasks/media-cache-gc.ts Updates task-level GC documentation to include the new “unreferenced” declared-slot reclamation arm.
packages/runtime-common/media-cache.ts Adds 'unreferenced' to GC reasons and implements the manifest-based unreferenced candidate predicate used in both CASE and WHERE.
packages/realm-server/tests/media-cache-gc-test.ts Adds targeted tests that exercise the new manifest-based reclamation behavior and its conservatism guards.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lukemelia
lukemelia requested review from a team and jurgenwerk September 11, 2026 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants