Skip to content

test(relay): pin delivery attempt retention cutoff - #23769

Closed
only21mil wants to merge 5 commits into
pingdotgg:mainfrom
only21mil:r13/b163-prune-old-relay-delivery-attempts-rows-i
Closed

only21mil wants to merge 5 commits into
pingdotgg:mainfrom
only21mil:r13/b163-prune-old-relay-delivery-attempts-rows-i

Conversation

@only21mil

@only21mil only21mil commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

relay delivery attempts need bounded retention without deleting recent deduplication records. main now provides the pruning service, five-minute cron, seven-day policy, batch cap, and created_at index.

this pr preserves that implementation and adds the original cutoff regression coverage to the existing batch test. it verifies both delete batches use the strict seven-day cutoff and a bounded subquery. no production code or ui differs from main.

verification at 0a759d515656ab3c424da704d8089617b27479e1:

  • blacksmith: 41 tests passed across DeliveryAttempts, ApnsDeliveries, and MobileRegistrations; relay typecheck passed.
  • blacksmith: vp run check:changed passed format, lint, both knip passes, relay typecheck, and all 6 DeliveryAttempts tests.
  • two independent gpt-6.1-sol reviewers found no issues on this exact head.
  • merged github/main without rebasing; final delta is one existing test file, 19 insertions and 3 deletions.

deployed cloudflare cron, planetscale, production volume, and apns execution remain unverified. no browser evidence applies to this test-only change.

updated with gpt-6.1-sol through the codex harness in t3 code.

only21mil and others added 2 commits October 22, 2026 12:16
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
relay_delivery_attempts gets a row for every APNs push attempt, and
nothing deleted from it. The five-minute cron now drops rows last
written more than 14 days ago. Source-job dedupe only needs the
10-minute signed job window, so the horizon is for diagnostics.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 97e35bc and 0a759d5.

Files selected for processing: 1
  • infra/relay/src/agentActivity/DeliveryAttempts.test.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

The pruning test helper now captures delete predicates. The short-batch test fixes the clock and checks the SQL and parameters for both delete batches, including the seven-day cutoff and batch-size limit.

Changes: Relay pruning test coverage

Layer / File(s) Summary
Capture and verify prune predicates
infra/relay/src/agentActivity/DeliveryAttempts.test.ts
The test helper captures each delete predicate. The short-batch test checks both rendered predicates against the expected subquery, seven-day cutoff, and batch-size limit.

Priority: Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related PRs

  • pingdotgg/t3code#26141: It also changes pruning for relay_delivery_attempts, with a different retention period and batch size.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 0a759

The test change is mergeable; it aligns coverage with the current seven-day pruning behavior.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the relay test change: pinning the delivery-attempt retention cutoff.
Description check Passed The description explains the retention problem, the test-only change, verification results, and unverified deployment checks. It does not use the template headings or provide an explicit issue or appr…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the retention problem, the test-only change, verification results, and unverified deployment checks. It does not use the template headings or provide an explicit issue or approval link, but it contains sufficient relevant information to be mostly complete.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 22, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7620b49

Macroscope's review found this PR approvable — Adds bounded five-minute pruning for relay delivery-attempt records older than 14 days, while preserving active delivery deduplication and using the existing cleanup error handling. The other changes are test-fake updates and a focused SQL predicate test; no product defaults or static-analysis overrides are changed.

You can add or adjust custom eligibility rules. Learn more.

@github-actions github-actions Bot added size:XXL 1,000+ changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

thanks @only21mil. the original cutoff, service method, cron wiring, and regression test are preserved. i merged current main and added only a created_at btree index with its generated postgres migration/snapshot, so the five-minute prune can find expired rows without scanning all retained attempts. no original pieces were dropped.

verification at 4ee85d950097f9172e5a93f87dad76d83e0d832a:

  • reproduced the regression against main's service: 1 failed (prune missing), 3 passed. restored the fix: 43 focused tests passed in blacksmith.
  • vp run check:changed passed in blacksmith: format, lint, both knip passes, relay typecheck, and 42 tests.
  • applied all 13 migrations to disposable postgres 18. exercised the real drizzle/effect service: expired row deleted; exact-cutoff row, recent completion, and active claim retained; completed/in-flight deduplication preserved; repeated prune succeeded.
  • postgres selected idx_relay_delivery_attempts_created_at for the delete predicate on 100,000 rows. this is index verification, not a production performance measurement.
  • all 28 reported ci checks passed. two independent sol reviewers inspected this exact head and found no issues. the automatic review was rate-limited; macroscope's existing approval is for the original head.

unverified: deployed worker cron execution, production table volume, initial cleanup duration, and planetscale-specific behavior. no ui changed, so visual evidence does not apply. disposable postgres and the task's blacksmith worker have been stopped.

maria-rcks commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

checked exact head 4ee85d950097f9172e5a93f87dad76d83e0d832a on a disposable linux vm. result: relay cleanup remains unverifiable, despite the successful checks below.

  • installed and ran current github/main (bd84b1781a) in a separate worktree, stopped its captured process group, then installed and ran the exact pr head with isolated state. only one dev server ran at a time.
  • used repository playwright and /usr/bin/google-chrome to add a real git repository through the app, select codex gpt-6.1-sol, submit a README tool-read request, and observe RELAY_VERIFY_OK Relay verification workspace. on both revisions.
  • on the pr, reloaded the conversation, submitted a follow-up, observed RELAY_FOLLOWUP_OK, then reloaded again. read-only inspection confirmed both persisted runs completed using gpt-6.1-sol. these prove neighboring provider/persistence behavior, not relay pruning.
  • blacksmith: the existing DeliveryAttempts, ApnsDeliveries, and MobileRegistrations suites passed, 42/42. relay typecheck passed. github reports no failing/pending head checks; preview deployments were skipped. github currently reports merge conflicts.

main's original missing retention could not be reproduced: https://git.cafe/pingdotgg/t3code/pulls/26141 already added cleanup in c3b40b97f7. current main retains seven days and deletes at most four batches of 5,000 rows per run. this pr retains fourteen days with one unbounded delete. i left the pr unchanged.

main, real codex turn:

main: real codex turn reads the verification repository

pr, same repository and prompt:

pr: completed gpt-6.1-sol turn with the same README response

animated recording of model selection, submission, tool execution, and completion. loading frames were trimmed; the source mp4's uploaded playback and seeking were checked:

pr: real gpt-6.1-sol tool-read turn

command and runtime transcript

Not verified

actual delivery-attempt retention, five-minute cron execution, apns delivery/deduplication, and production cleanup duration. vp run dev starts the environment server and web client, not the cloudflare relay. both servers logged agent activity publishing standby; waiting for T3 Connect link reconciliation. this vm has no configured relay/Clerk/APNs/cloud provisioning credentials or registered ios push device. the pr's Connections screen exposes no T3 Connect sign-in. no synthetic rows or mocks were substituted for that path.

pr: connections without configured t3 connect

native ios/android, macos/windows desktop, deployed cloudflare/planetscale behavior, and production volume remain unverified. a real isolated relay plus account/device credentials and legitimately aged delivery rows are needed to complete that verification. no source changes or pushes were made.

@maria-rcks maria-rcks changed the title fix(relay): stop relay_delivery_attempts from growing forever test(relay): pin delivery attempt retention cutoff Oct 30, 2026
@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XXL 1,000+ changed lines (additions + deletions). labels Oct 30, 2026
@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 GPT-6.1 Sol responding on behalf of Victor

Closing in favor of #26141, which landed delivery-attempt retention and bounded cleanup. The remaining diff here only adds an exact cutoff/SQL-parameter assertion; that extra test has not landed on main.

@only21mil only21mil closed this Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants