Skip to content

Sweep turn screenshots in every deployment, not only where a culler runs - #264

Open
zopeVaibhav wants to merge 1 commit into
CopilotKit:mainfrom
zopeVaibhav:fix/page-frames-never-purged
Open

Sweep turn screenshots in every deployment, not only where a culler runs#264
zopeVaibhav wants to merge 1 commit into
CopilotKit:mainfrom
zopeVaibhav:fix/page-frames-never-purged

Conversation

@zopeVaibhav

Copy link
Copy Markdown
Contributor

Closes #263.

What this changes

computer_page_frame is written on every navigation in every deployment, and its reaper had one caller: scripts/cull-idle-computers.ts, which refuses to run unless the provider is sandbox and is scheduled only by the chart's culler CronJob, which renders only in that mode. So Compose, the all-in-one image and the chart's own default of computers.mode: shared wrote a row per navigation and removed none.

The sweep moves to the server, onto the hourly timer that already removes old audit rows — one timer for both, which is the "second thing to get wrong" the cull script was right to avoid. It runs whether or not AUDIT_RETENTION_DAYS is set, because the thirty-day window belongs to the store rather than to the audit policy, and gating it on that flag would leave most deployments exactly where they are.

purge is now batched. It was one statement with returning, which is fine in a CronJob and not fine on an ordinary server.

The culler no longer purges frames. The server does it there too, and one owner beats two.

The upgrade consequence, stated plainly because it is the one surprise here. Every deployment that is not computers.mode: sandbox has been keeping every screenshot since it was installed. The first sweep after this lands removes the ones older than thirty days, about a minute after the server starts. That is the window the store has always documented and the one sandbox deployments already enforce, but it has never been applied anywhere else. If it should be opt-in for existing installations rather than on, say so and I will put it behind a value — I have taken the view that a table nothing bounds is the worse default, and that reusing AUDIT_RETENTION_DAYS for it would leave most deployments exactly where they are.

Where it runs

  • New state that outlives a request? None. No new table, column or in-process state; the store is now built once and shared rather than three times.
  • What happens on the second replica? Every replica runs the timer, and the delete is idempotent — a row removed by one is simply not there for the next. The audit half keeps its advisory lock, which is what stops N replicas doing the same large delete; the frame half is bounded per tick instead, at 200 rows a statement and 200 statements a sweep.
  • Anything serialised? The audit sweep, by the advisory lock it already had. The frame sweep does not need it: a delete ... where captured_at < now() - interval run twice removes the same rows once, which is the same reasoning cull-idle-computers.ts gives for why retention may run everywhere and suspension may not.
  • Anything fanned out to a browser? No.
  • New listener, port, or schedule? No new timer. The existing retention timer now starts when either sweep is wanted rather than only when audit retention is configured.

Boundary and audit

  • Every acting call still goes through the gateway: the gateway is unchanged except that it receives the shared store instead of its own.
  • New refusals and new failures each write a row: none added. A failed frame sweep logs page-frames-sweep-failed and leaves the rows, matching the audit sweep beside it.
  • Nothing new is trusted from the client: no new input.

The append-only audit trigger is untouched — this table is not the audit trail and has no such trigger.

Changelog

  • A line in CHANGELOG.md under Unreleased.

Proof

Against a real database. One Bot, forty navigations a day for ninety days, 200 KB a frame — random bytes, because base64 of a real PNG is near-incompressible and a repeated string is not, which made a first attempt look twenty times smaller than the truth:

  rows: 3600   table: 734 MB

On main, in a Compose-shaped deployment with AUDIT_RETENTION_DAYS unset:

  rows written, two of them older than 30 days: 3
  after main's scheduler ran:                   3   <- nothing swept

With this branch, same shape, same unset flag:

  rows before:        3600
  {"type":"page-frames-swept","removed":2440}
  rows after sweep:   1160
  older than 30 days: 0

Batching, on the 734 MB table:

  before   purge removed 2440 rows in 17.6s     one statement, locks held throughout
  after    purge removed 2440 rows in  6.5s     200 rows a statement

Repository checks:

  • bun run typecheck in server/: clean.
  • bunx biome check on every touched file: clean.
  • bun test over the retention, page-frame, audit and culler suites: 47 pass, 0 fail, run four times consecutively to be sure the polling test added here is not flaky.
  • server/tests/page-frame-retention.integration.test.ts is new. Its first case passes undefined for the audit window, which is what a deployment that never configured retention has, and is the case that previously returned before starting a timer at all.

@zopeVaibhav
zopeVaibhav force-pushed the fix/page-frames-never-purged branch from 690c1e3 to 200c88e Compare August 26, 2026 18:47
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.

Turn screenshots are written in every deployment and swept in one

1 participant