feat: add opt-in event retention purge (#359) - #412
Conversation
cameri
commented
Apr 9, 2026
Please recheck the deletion conditions. There's a query that potentially wipes the events table. That would be catastrophic. |
cameri
left a comment
There was a problem hiding this comment.
I think some changes are required to move forward, but mostly looking pretty good!
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
49044d2 to
7173f4dCompareJustxd22
commented
Apr 9, 2026
returnthis.masterDbClient('events').where(function(){this.where(function(){this.whereNotNull('expires_at').andWhere('expires_at','<',now)}).orWhereNotNull('deleted_at').orWhere('event_created_at','<',retentionLimit)}).del()i've missed the deletion conditions BUG during my tests as i've been adding only 1 event and testing with it, Good catch @cameri ! |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
cameri
commented
Apr 10, 2026
Looking good! Left just a few comments. |
There was a problem hiding this comment.
Pull request overview
Adds an opt-in event retention/purge mechanism that runs from the existing MaintenanceWorker heartbeat, enabling operators to automatically delete expired (NIP-40), soft-deleted (NIP-09), and older-than-retention events via a repository-level delete operation.
Changes:
- Introduces
limits.event.retentionDayssetting (default disabled) and documents it. - Adds
MaintenanceService.clearOldEvents()and wires it intoMaintenanceWorkerschedule loop. - Implements
EventRepository.deleteExpiredAndRetained()and adds unit tests for worker/service/repository behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/services/maintenance-service.spec.ts | Adds unit tests for retentionDays gating and error handling in MaintenanceService. |
| test/unit/repositories/event-repository.spec.ts | Adds query/behavior tests for deleteExpiredAndRetained. |
| test/unit/app/maintenance-worker.spec.ts | Adds unit tests ensuring the worker calls maintenance even when payments are disabled. |
| src/services/maintenance-service.ts | New service that reads settings and triggers event purging. |
| src/repositories/event-repository.ts | Adds a single delete query handling expiration, soft-deletes, and retention cutoff. |
| src/factories/maintenance-worker-factory.ts | Wires MaintenanceService into MaintenanceWorker. |
| src/factories/maintenance-service-factory.ts | New factory for constructing MaintenanceService with EventRepository. |
| src/app/maintenance-worker.ts | Invokes maintenanceService.clearOldEvents() on each schedule tick. |
| src/@types/settings.ts | Adds retentionDays?: number to event limits settings. |
| src/@types/services.ts | Adds IMaintenanceService interface. |
| src/@types/repositories.ts | Adds deleteExpiredAndRetained() to IEventRepository. |
| resources/default-settings.yaml | Adds default limits.event.retentionDays: -1 (disabled). |
| CONFIGURATION.md | Documents limits.event.retentionDays behavior and default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in event retention/expiry purge to reduce long-term storage and index load by periodically deleting expired (NIP-40), soft-deleted (NIP-09), and retention-aged events as part of the existing maintenance heartbeat.
Changes:
- Introduces
limits.event.retentionDaysconfiguration (default disabled) and documents it. - Adds a maintenance service and wires it into
MaintenanceWorker’s scheduled loop. - Implements a single-query purge method in
EventRepositoryand adds unit tests around the new behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/services/maintenance-service.spec.ts | Adds unit coverage for retentionDays gating and error handling in the maintenance service. |
| test/unit/repositories/event-repository.spec.ts | Adds unit coverage asserting generated SQL and retentionDays guard behavior for the purge query. |
| test/unit/app/maintenance-worker.spec.ts | Adds unit coverage ensuring maintenance runs regardless of payments being enabled. |
| src/services/maintenance-service.ts | Implements MaintenanceService.clearOldEvents() with retentionDays gating and logging. |
| src/repositories/event-repository.ts | Adds deleteExpiredAndRetained() delete query combining retention, expiry, and soft-deletes. |
| src/factories/maintenance-worker-factory.ts | Wires the new maintenance service into the worker factory. |
| src/factories/maintenance-service-factory.ts | Adds factory to construct MaintenanceService with an EventRepository. |
| src/app/maintenance-worker.ts | Invokes maintenance purge on the existing schedule before invoice processing. |
| src/@types/settings.ts | Extends EventLimits with retentionDays?: number. |
| src/@types/services.ts | Adds IMaintenanceService interface. |
| src/@types/repositories.ts | Extends IEventRepository with deleteExpiredAndRetained(). |
| resources/default-settings.yaml | Adds limits.event.retentionDays: -1 default (disabled). |
| CONFIGURATION.md | Documents the new limits.event.retentionDays setting. |
Comments suppressed due to low confidence (1)
src/app/maintenance-worker.ts:44
clearOldEvents()is awaited before invoice processing. If the purge DELETE takes a long time, it will delay invoice updates and can also increase the chance that overlappingonSchedule()runs occur (sincesetIntervaldoesn't wait). Consider adding a re-entrancy guard (skip if a run is in progress) and/or running purge and invoice checks concurrently so invoice processing isn't blocked by a large purge.
private async onSchedule(): Promise<void> {
const currentSettings = this.settings()
await this.maintenanceService.clearOldEvents()
if (!path(['payments','enabled'], currentSettings)) {
return
}
const invoices = await this.paymentsService.getPendingInvoices()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
cameri
commented
Apr 10, 2026
Please address Copilot comments |
7173f4d to
bde7cdeCompareThere was a problem hiding this comment.
Pull request overview
This PR adds an opt-in event retention purge that runs from the existing MaintenanceWorker heartbeat, deleting batches of expired (NIP-40), soft-deleted (NIP-09), and older-than-retention events via a single repository query.
Changes:
- Introduces
MaintenanceService.clearOldEvents()and wires it intoMaintenanceWorker’s scheduled loop. - Adds
EventRepository.deleteExpiredAndRetained()to batch-delete up to 1000 qualifying events per run, with optional kind/pubkey whitelists. - Adds settings/types/docs for
limits.event.retention.maxDaysplus unit tests covering worker/service/repository behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/services/maintenance-service.spec.ts | Adds unit tests for retention-driven purge orchestration + logging/error handling. |
| test/unit/repositories/event-repository.spec.ts | Adds query-string tests for the new batched purge query and whitelist behavior. |
| test/unit/app/maintenance-worker.spec.ts | Ensures scheduled loop always calls maintenance, regardless of payments config. |
| src/services/maintenance-service.ts | New service that reads retention config and triggers repository purge. |
| src/repositories/event-repository.ts | Implements the batched delete query + purge count mapping. |
| src/factories/maintenance-worker-factory.ts | Wires MaintenanceService into the worker factory. |
| src/factories/maintenance-service-factory.ts | New factory creating MaintenanceService + EventRepository. |
| src/app/maintenance-worker.ts | Adds maintenance execution to the schedule + overlap protection. |
| src/@types/settings.ts | Adds retention config types under limits.event.retention. |
| src/@types/services.ts | Introduces IMaintenanceService. |
| src/@types/repositories.ts | Adds purge options/counts types + repository method signature. |
| resources/default-settings.yaml | Adds default retention config (disabled via maxDays: -1). |
| CONFIGURATION.md | Documents new retention settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
bde7cde to
57dad0fCompareThere was a problem hiding this comment.
Pull request overview
Adds an opt-in maintenance feature to purge events that are expired (NIP-40), soft-deleted (NIP-09), or older than a configured retention window, wired into the existing MaintenanceWorker heartbeat.
Changes:
- Introduces
MaintenanceService.clearOldEvents()and wires it intoMaintenanceWorker’s scheduled loop. - Adds
EventRepository.deleteExpiredAndRetained()with a batch-limited delete query and whitelist exclusions. - Extends settings/types/docs/default settings and adds unit tests for worker/service/repository behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/services/maintenance-service.spec.ts | Adds unit tests for retention-enabled/disabled and error handling paths. |
| test/unit/repositories/event-repository.spec.ts | Adds SQL-string tests for the purge delete query and whitelist behavior. |
| test/unit/app/maintenance-worker.spec.ts | Adds unit tests ensuring scheduled runs invoke maintenance and respect payments enabled/disabled. |
| src/services/maintenance-service.ts | New service implementing settings-driven purge orchestration and logging. |
| src/repositories/event-repository.ts | Implements batch purge query + returned counts for deleted/expired/retained rows. |
| src/factories/maintenance-worker-factory.ts | Wires MaintenanceWorker with the new MaintenanceService dependency. |
| src/factories/maintenance-service-factory.ts | New factory to construct MaintenanceService with EventRepository + settings provider. |
| src/app/maintenance-worker.ts | Schedules maintenance purge each interval and adds an isRunning guard against overlap. |
| src/@types/settings.ts | Adds typed settings shape for limits.event.retention.*. |
| src/@types/services.ts | Adds IMaintenanceService. |
| src/@types/repositories.ts | Adds retention option and purge-count types and repository method contract. |
| resources/default-settings.yaml | Adds default (disabled) retention config and default kind whitelist. |
| CONFIGURATION.md | Documents the new retention settings keys and behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
57dad0f to
3b3abf0Compare
limits.event.retentionDaystosettings.yaml.retentionDaysis unset/non-positiveDescription
The logic is hooked into the existing heartbeat of the MaintenanceWorker:
Add the following to your
.nostr/settings.yaml:Related Issue
#359
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: