Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds tenant-configured retention awareness across API responses and the frontend UI, exposing dataRetentionPeriod and showing a retention banner when users request data outside the allowed window.
Changes:
- Expose
dataRetentionPeriodon the Tenant API model (OpenAPI + Go/TS generated clients) and populate it from SQL. - Add backend retention helpers + analytics signals when requests are outside retention.
- Add frontend retention utilities and UI banners for runs/logs/events, plus expanded time-window presets.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/repository/sqlcv1/tenants.sql.go | sqlc regen to include tenant dataRetentionPeriod in PopulateTenantMembers rows. |
| pkg/repository/sqlcv1/tenants.sql | Adds dataRetentionPeriod to the PopulateTenantMembers select list. |
| pkg/client/rest/gen.go | Adds Tenant.dataRetentionPeriod to the generated Go REST client model. |
| frontend/app/src/pages/main/v1/workflow-runs-v1/hooks/use-runs-table-filters.tsx | Expands supported time-window presets and simplifies time math. |
| frontend/app/src/pages/main/v1/workflow-runs-v1/components/runs-table.tsx | Shows retention empty-state banner and suppresses rows when outside retention; switches tenant-id sourcing. |
| frontend/app/src/pages/main/v1/workflow-runs-v1/$run/index.tsx | Shows a retention overlay/banner when an individual run is outside retention. |
| frontend/app/src/pages/main/v1/logs/index.tsx | Shows a retention overlay/banner when log time window is outside retention. |
| frontend/app/src/pages/main/v1/events/index.tsx | Shows retention empty-state banner and suppresses rows when outside retention. |
| frontend/app/src/lib/utils/retention.ts | New retention parsing/boundary helpers for the UI. |
| frontend/app/src/lib/api/generated/data-contracts.ts | Adds Tenant.dataRetentionPeriod to generated TS contracts. |
| frontend/app/src/components/v1/ui/textarea.tsx | Formatting-only change to props interface declaration. |
| frontend/app/src/components/v1/ui/card.tsx | Formatting-only change to props interface declaration. |
| frontend/app/src/components/v1/ui/button.tsx | Formatting-only change to props interface declaration. |
| frontend/app/src/components/v1/ui/badge.tsx | Formatting-only change to props interface declaration. |
| frontend/app/src/components/v1/shared/duration.tsx | Formatting-only change to props interface declaration. |
| frontend/app/src/components/v1/retention-banner.tsx | New shared banner component for Cloud vs OSS retention messaging. |
| frontend/app/src/components/v1/molecules/time-picker/time-picker-input.tsx | Formatting-only change to props interface declaration. |
| frontend/app/src/components/v1/molecules/data-table/data-table-options.tsx | Adds 3d/14d/30d time-window options to the time-window selector. |
| frontend/app/src/components/v1/molecules/data-table/data-table-column-header.tsx | Formatting-only change to generic props interface declaration. |
| api/v1/server/oas/transformers/user.go | Includes dataRetentionPeriod in tenant info returned with tenant members. |
| api/v1/server/oas/transformers/tenant.go | Includes dataRetentionPeriod in standalone tenant responses. |
| api/v1/server/oas/gen/openapi.gen.go | Regenerates OpenAPI server types and embedded swagger spec for the new field. |
| api/v1/server/handlers/v1/workflow-runs/list.go | Adds retention checks + analytics signals on workflow-run list endpoints. |
| api/v1/server/handlers/v1/workflow-runs/get.go | Adds retention checks + analytics signals on workflow-run get endpoint. |
| api/v1/server/handlers/v1/tasks/get.go | Adds retention checks + analytics signals on task-run get endpoint. |
| api/v1/server/handlers/v1/retention.go | New shared backend helper to evaluate retention boundaries. |
| api/v1/server/handlers/v1/logs/list.go | Adds retention checks + analytics signals on log list endpoint. |
| api/v1/server/handlers/v1/events/list.go | Adds retention checks + analytics signals on event list endpoint. |
| api/v1/server/handlers/v1/events/get.go | Adds retention checks + analytics signals on event get endpoint. |
| api-contracts/openapi/components/schemas/tenant.yaml | Adds dataRetentionPeriod to the Tenant schema. |
| if v1handlers.IsBeforeRetention(since, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx, analytics.WorkflowRun, analytics.List, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| if ts := rawWorkflowRun.WorkflowRun.CreatedAt; ts.Valid && v1handlers.IsBeforeRetention(ts.Time, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx.Request().Context(), analytics.WorkflowRun, analytics.Get, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
This handler currently only tracks an analytics signal when the run is outside retention, but still returns the run details. If retention should be enforced at the API layer, consider returning a 404/410 (or a structured error) when the run is older than the tenant’s retention period.
| if ts := task.InsertedAt; ts.Valid && v1handlers.IsBeforeRetention(ts.Time, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx.Request().Context(), analytics.TaskRun, analytics.Get, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| if since != nil && v1handlers.IsBeforeRetention(*since, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx.Request().Context(), analytics.Log, analytics.List, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| const tableColumns = useMemo( | ||
| () => | ||
| columns( | ||
| tenantId, | ||
| tenantId!, | ||
| selectedAdditionalMetaRunId, |
There was a problem hiding this comment.
useAppContext() can return tenantId as undefined while tenant memberships are still loading, but this code asserts non-null (tenantId!). That can crash on initial render. Consider reading the tenant id from route params here (like GetWorkflowChart already does), or rendering a loading/empty state until tenantId is available.
| if v1handlers.IsBeforeRetention(since, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx, analytics.WorkflowRun, analytics.List, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| if ts := rawWorkflowRun.WorkflowRun.CreatedAt; ts.Valid && v1handlers.IsBeforeRetention(ts.Time, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx.Request().Context(), analytics.WorkflowRun, analytics.Get, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| if v1handlers.IsBeforeRetention(since, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx.Request().Context(), analytics.Event, analytics.List, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| if ts := event.EventSeenAt; ts.Valid && v1handlers.IsBeforeRetention(ts.Time, tenant.DataRetentionPeriod) { | ||
| t.config.Analytics.Count(ctx.Request().Context(), analytics.Event, analytics.Get, analytics.Properties{ | ||
| "outside_retention": true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
For Count() instrumentation, boolean properties should follow the repo convention of has_* (to keep dashboards consistent). Consider renaming outside_retention to something like has_retention_violation (and apply consistently across all retention events).
| const defaultRetention = 720 * time.Hour | ||
|
|
||
| func retentionBoundary(retentionPeriod string) time.Time { | ||
| retention, err := time.ParseDuration(retentionPeriod) | ||
| if err != nil || retention <= 0 { | ||
| retention = defaultRetention | ||
| } | ||
|
|
||
| return time.Now().Add(-retention) |
There was a problem hiding this comment.
retentionBoundary falls back to a hard-coded 720h when parsing fails. The rest of the server already has a configurable default retention (Runtime.Limits.DefaultTenantRetentionPeriod) and the DB default may also evolve; hard-coding here risks inconsistent behavior if defaults change. Consider taking the default from config (or treating invalid/empty retention as “no enforcement” and returning false).
Expands the data retention page to explain: - What data gets deleted and when - The UI banner users see when accessing old data - That in-progress runs are never deleted Related to PR hatchet-dev#3615 which implements retention enforcement.
|
Promptless prepared a documentation update related to this change. Triggered by this PR Expanded the data retention documentation to explain what gets deleted (completed/failed/cancelled workflow runs, events, and associated logs older than the retention period), that in-progress runs are never deleted, and how users see warning banners when viewing data outside their retention window. Review: Document retention enforcement |
|
Cloud: 
OSS: 
|
Description
Enforces retention per exisiting plan configuration
Type of change