Conversation
Wire database.Chat into the audit system so chat lifecycle events
(manual patches, background auto-archives, deletions) produce audit
log entries.
- Add 'chat' value to the resource_type enum (migration 000473).
- Register Chat in the Auditable union, AuditActionMap, and
AuditableResources with per-field tracking that ignores churny
internal fields (status, heartbeat_at, updated_at,
last_model_config_id, ...) and tracks user-meaningful fields
(title, archived, pin_order, mode, labels, mcp_server_ids, ...).
- Implement ResourceTarget/ResourceID/ResourceType/ResourceRequiresOrgID
cases for database.Chat.
- Implement auditLogIsResourceDeleted via GetChatByID (chats are
hard-deleted; missing row == deleted).
- Implement auditLogResourceLink -> /chats/{id}.
- Wire audit.InitRequest[database.Chat] into the patchChat handler
so manual updates (title, archived, pin_order, labels, workspace
binding) produce audit records.
- Introduce BackgroundSubsystemChatAutoArchive for the upcoming
dbpurge integration, and broaden the background-audit log line
from 'dormancy audit' to 'background audit'.
- Regenerate audit-logs.md, swagger, schemas.md, and typesGenerated.ts.
Part of CODAGT-200.
6018381 to
ad22b73
Compare
9fd96ff to
e4a0406
Compare
e4a0406 to
a18186c
Compare
Adds the schema and queries needed to auto-archive inactive chats from dbpurge, following the plan in /tmp/plans/chat-auto-archive.md (not wired up yet — that's Phase 3). * New migration 000471 creates the per-owner digest dedupe log. * New chats.sql query AutoArchiveInactiveChats drives the whole show from a LATERAL MAX(chat_messages.created_at) over each family. All roles count; soft-deleted messages do not. * New siteconfig.sql queries for the 'agents_chat_auto_archive_days' knob, default 90, 0 disables. Mirrors the existing retention knob. * New chat_auto_archive_digest_log.sql for batch read + upsert of the per-owner dedupe rows. The actual dbpurge wiring, HTTP endpoints, notification template, and dispatch logic land in subsequent commits.
Exposes the chat auto-archive knob introduced in the previous commit via `GET` / `PUT /api/experimental/chats/config/auto-archive-days`, mirroring the existing retention-days endpoint. * New codersdk.ChatAutoArchiveDaysResponse and UpdateChatAutoArchiveDaysRequest types plus ExperimentalClient methods. * getChatAutoArchiveDays is readable by any authenticated user (same as retention). putChatAutoArchiveDays requires policy.ActionUpdate on rbac.ResourceDeploymentConfig. * Validation: 0 <= days <= 3650. 0 disables auto-archive. * TestChatAutoArchiveDays covers the defaults, admin/member auth split, disable (0), aggressive (1), and the two rejection cases. The value is still not being read by anything — dbpurge wiring comes later.
Adds the single notification template that dbpurge will enqueue when it auto-archives one or more of a user's chats. One notification per owner per day (enforced by the dedupe log added earlier), listing the chats that were archived. * Migration 000472 inserts the template with group 'Chat Events'. * Template UUID registered in coderd/notifications/events.go under a new 'Chat-related events' var block. * Golden SMTP + webhook renders committed as fixtures so future copy changes are reviewable. * inboxnotifications.go falls back to the 'Other' icon (we don't currently have a chat-specific inbox icon).
Wire the AutoArchiveInactiveChats query into the existing dbpurge
tick so chats inactive for \`chat_auto_archive_days\` (default 90)
are automatically archived. Owners receive at most one digest
notification per 24 h listing the titles of their newly-archived
chats; pinned chats and chats belonging to a still-active
conversation (non-null parent) are exempt.
- Extend dbpurge.New signature with audit.Auditor and
notifications.Enqueuer. Update the cli/server.go call site and
all 20 test call sites. Tests that use dbmock gain an AnyTimes
expectation for the new GetChatAutoArchiveDays read.
- Read chat_auto_archive_days pre-tx (mirroring
chat_retention_days). A 0 value disables the feature entirely.
- Inside the purge tx, after the existing delete pass, run
AutoArchiveInactiveChats bounded by chatsAutoArchiveBatchSize
(1000 roots per tick). Use dbauthz.AsSystemRestricted so the
UPDATE is permitted (AsDBPurge lacks chat-write capability).
- Post-commit, emit one BackgroundAudit[database.Chat] per archived
row (root + cascaded children), then build per-owner digests
containing only roots. Dedupe against
chat_auto_archive_digest_log with a 24 h window, enqueue via
TemplateChatAutoArchiveDigest, and upsert the dedupe timestamp
only after a successful enqueue so transient failures retry.
- Surface two Prometheus counters:
coderd_chat_auto_archive_records_archived_total and
coderd_chat_auto_archive_digests_sent_total{outcome} (enqueued,
skipped_deduped, skipped_dedupe_error, enqueue_failed).
- Extend the AutoArchiveInactiveChats SQL to return
last_activity_at; cascaded children inherit the root's value via
COALESCE so the scan column never sees NULL.
- Add TestAutoArchiveInactiveChats covering:
AutoArchiveDisabled, ArchivesInactiveRoot,
SkipsPinnedAndChildren, DedupesDigestWithin24h.
Closes CODAGT-199.
Add a dedicated platform controls page covering the new auto-archive behaviour, its interaction with the existing retention window, pinned-chat exemption, and the once-per-24h digest notification. Cross-link from the retention page and register the new path in docs/manifest.json. Part of CODAGT-199.
Fold deep-review feedback: - P1 (security/contract): getChatAutoArchiveDays now requires policy.ActionUpdate on ResourceDeploymentConfig before returning the value. Mirrors getChatSystemPrompt. The endpoint surface was previously readable by any authenticated user, leaking a deployment-wide configuration knob to non-admins. Update the existing endpoint test to assert the non-admin forbidden path instead of the happy-path read. - P2 (concurrency): detach the post-commit dispatch context from the ticker context with context.WithoutCancel and a 30s deadline. Canceling the ticker mid-dispatch (server shutdown) could previously split an owner's enqueue from their dedupe-log upsert, causing a duplicate digest on the next tick. The deadline bounds graceful shutdown while still allowing in-flight owners to finish. - P2 (performance): add a partial index on chats covering the AutoArchiveInactiveChats WHERE clause (archived = false, pin_order = 0, parent_chat_id IS NULL) so the CTE can jump straight to candidate roots instead of scanning the full chats table. Existing idx_chat_messages_chat_created already covers the LATERAL MAX(created_at) subquery. Also folds the make gen indentation fixup to docs/manifest.json.
…igest_log TestMigrateUpWithFixtures asserts that every table has at least one row after running through all migrations. Migration 000473 creates chat_auto_archive_digest_log without seeding any data, so the test flagged it as empty. Seed a single dedupe row against the first available user so the table is covered.
…ions into one
Collapse 000473 (digest log table), 000474 (digest notification
template), and 000475 (partial index on chats) into a single
000473_chat_auto_archive.{up,down}.sql. Reviewers only have to
reason about one file; the three pieces are tightly coupled to the
same feature anyway.
notification_messages already deduplicates on (template_id, user_id, method, payload, day) via an MD5 hash trigger (migration 000245), which is exactly the guarantee our custom chat_auto_archive_digest_log table was providing - and users who find the digest noisy can disable the template entirely from their notification preferences. The custom 24h sliding window was only measurably better than native dedupe during a large initial backfill where the same owner gets hit across many ticks with payloads that happen to differ. That scenario needs a deployment with thousands of stale chats to matter; under early-access adoption (feature off by default, customers hop on deliberately), it does not. - Drop migration of chat_auto_archive_digest_log from 000473. - Drop GetChatAutoArchiveDigestLogsForOwners and UpsertChatAutoArchiveDigestLog queries, their dbauthz wrappers, and the dbauthz test cases. - Simplify dispatchChatAutoArchive to just audit + enqueue per owner; drop the unused db parameter. - Drop chatAutoArchiveDigestDedupe constant and the skipped_deduped / skipped_dedupe_error Prometheus labels. - Drop the migration fixture (table no longer exists). - Drop the DedupesDigestWithin24h dbpurge subtest and the dedupe-log assertion inside ArchivesInactiveRoot. - Update the auto-archive docs to describe native per-day dedupe and the per-template opt-out. Net: -288 LoC.
- Drop coderd_chat_auto_archive_digests_sent_total. The native
coderd_notifications_inflight_dispatches and
coderd_notifications_retry_count are both labeled with
notification_template_id, so filtering by the chat auto-archive
template ID gives the same data without an extra per-feature
metric. Only coderd_chat_auto_archive_records_archived_total
stays because nothing else counts chats archived per tick.
- Fix the duplicate greeting in the digest template. The SMTP and
webhook wrappers already prepend 'Hi {{.UserName}},', so the
body_template must not repeat it.
- Drop the 'View archived chats' action button. The
/chats?archived=true route isn't supported in the frontend yet
and adding that view is out of scope for this PR.
…count Deep review round 2 follow-up. - Drop stale dedupe-log references from constant, function, and dispatch-block comments. The custom dedupe table was removed in 5831e0d; comments now say what the code actually does. - Remove unused int32 parameters from buildDigests. They were retained as blank identifiers during simplification but never used; moved to buildDigestData which is the real consumer. - Compress verbose comments on constants, helpers, and types without losing meaningful content. dbpurge.go: 566 -> 544 LoC. - Render additional_archived_count in the digest body template. buildDigestData already computes the overflow count, but the template never consumed it, so a user archiving 100 chats at once would silently see only the first 25 titles with no hint more existed. Now the body ends with "...and N more." when applicable.
Deep review round 3. - Rename chatsAutoArchiveBatchSize to chatAutoArchiveBatchSize so all four chatAutoArchive* constants share a consistent naming convention (flagged by go-architect as a minor inconsistency). - Add TestAutoArchiveInactiveChats/DigestOverflowCap. The overflow branch of buildDigestData (cap titles at 25, surface extras via additional_archived_count) was untested; this archives 27 roots and asserts the digest payload caps at 25 titles with a "2" overflow count.
… auth Address review feedback: - Drop swagger annotations from the auto-archive-days handlers; document the endpoints in docs/ai-coder/agents/chats-api.md instead. Swagger will pick this up when the API stabilizes. - Grant UPDATE+READ on rbac.ResourceChat to subjectDBPurge and drop AsSystemRestricted from the auto-archive call; the ticker's AsDBPurge ctx now has everything it needs. - Replace the custom labelsFromRaw helper with StringMap.Scan, which already handles []byte and nil. - Compress the SQL comment block on AutoArchiveInactiveChats into a three-line header plus short inline annotations on the non-obvious predicates. - Trim verbose Go docstrings across dbpurge.go: New, the dispatch block, chatFromAutoArchiveRow, dispatchChatAutoArchive, buildDigests, buildDigestData, and the constant block.
…ctiveChats The comment referenced subjectSystemRestricted, but dbpurge now runs with subjectDBPurge (which was granted ResourceChat read+update in the same round).
…s retention clock DeleteOldChats and DeleteOldChatFiles both key off chats.updated_at, which AutoArchiveInactiveChats bumps to NOW() on archive (mirroring manual archive). Users/admins deserve to know the clock restarts. Follow-up tracked in CODAGT-222: add an archived_at column so the purge queries can stop using updated_at as a proxy.
a18186c to
015ee07
Compare
Renumber 000473_chat_auto_archive to 000474 after main landed 000473_mcp_server_allow_in_plan_mode. Conflict resolutions: - coderd/audit.go: take main's terser delete detection and the /agents/ route. - coderd/exp_chats.go: take main's inlined *api.Auditor.Load() and its post-update DB refresh before stamping aReq.New. - coderd/exp_chats_test.go: keep TestChatAutoArchiveDays and main's new Title subtest block; take main's nolint comment on TestUserChatCompactionThresholds. - enterprise/audit/table.go: take main's Chat action map (create, write) and plan_mode = ActionIgnore. - docs/admin/security/audit-logs.md: regenerated. - docs/ai-coder/agents/chats-api.md: keep Configuration section; take main's status table (adds requires_action). Also strip emdashes from lines surfaced by the new lint/emdash check (main added scripts/check_emdash.sh since our merge base): three in coderd/x/chatd/chatd.go and three in our docs.
Adds three subtests to TestAutoArchiveInactiveChats: - MultipleOwners: two owners in one org each get their own digest with only their chats; audit count matches archived roots. - SecondTickIdempotent: after tick 1 archives two roots, a third root seeded between ticks archives on tick 2 without re-archiving or re-notifying the first two. - BatchSizePagination: with batch size 20 and 27 stale roots, tick 1 archives 20, tick 2 archives the remaining 7, tick 3 is a no-op. To drive multi-tick tests deterministically, introduce a tickDriver helper that installs quartz traps *before* dbpurge.New so the forced initial tick and each loop iteration can be observed without racing with asynchronous ticker.Reset calls. tickDriver is separate from awaitDoTick because awaitDoTick conflates the forced tick and the first loop iteration, which is fine for idempotent single-tick tests but hides per-tick state when batch size is limiting. chatAutoArchiveBatchSize becomes a var (from const) so SetChatAutoArchiveBatchSizeForTest in export_test.go can shrink it for pagination tests without inserting thousands of rows.
|
/coder-agents-review |
There was a problem hiding this comment.
First-pass review (Netero only). 1 P2, 1 Nit. The full review panel has not yet reviewed this PR; it will review after these findings are addressed.
The auto-archive plumbing is well-structured: good test coverage ratio, correct batch pagination with idempotent second-tick verification, clean RBAC grant scoping on the dbpurge subject, and a sensible digest overflow cap. The chatFromAutoArchiveRow mapper and site_config pattern follow established conventions.
1 P2: notification template renders misleadingly when retention is disabled (retention_days=0).
1 Nit: dbauthz comment inaccurately describes access control.
"The notification tells users their data will be immediately deleted when it will not." (Netero)
🤖 This review was automatically generated with Coder Agents.
| '764031be-4863-4220-867b-6ce1a1b7a5f5', | ||
| 'Chats Auto-Archived', | ||
| E'Chats auto-archived after {{.Data.auto_archive_days}} days of inactivity', | ||
| E'The following chat{{if ne (len .Data.archived_chats) 1}}s were{{else}} was{{end}} automatically archived because {{if ne (len .Data.archived_chats) 1}}they have{{else}}it has{{end}} been inactive for more than {{.Data.auto_archive_days}} days:\n\n{{range .Data.archived_chats}}* "{{.title}}" (last active {{.last_activity_humanized}})\n{{end}}{{with .Data.additional_archived_count}}...and {{.}} more.\n\n{{end}}\nYou can restore any of them from the Chats page within {{.Data.retention_days}} days, after which they will be permanently deleted.', |
There was a problem hiding this comment.
P2 [DEREM-1] The body template ends with within {{.Data.retention_days}} days, after which they will be permanently deleted. When retention_days is 0 (retention disabled), this renders as "within 0 days, after which they will be permanently deleted." Per the docs, retention_days=0 means archived chats are kept indefinitely. An admin can configure auto_archive_days=90, retention_days=0 to archive without deleting, and every affected user then receives a notification claiming their data will be immediately deleted when it will not.
buildDigestData already passes retention_days as a string, so the template can branch: {{if eq .Data.retention_days "0"}}Archived chats will be kept indefinitely.{{else}}You can restore any of them from the Chats page within {{.Data.retention_days}} days, after which they will be permanently deleted.{{end}} (Netero)
🤖
There was a problem hiding this comment.
🤖
addressed (6bea837): Branched the template body on retention_days="0" so the digest now says "You can restore any of them from the Chats page; archived chats are kept indefinitely." when retention is disabled. Added a second golden fixture TemplateChatAutoArchiveDigestRetentionZero to lock that branch in (both smtp and webhook renderings).
| return q.db.GetAuthorizationUserRoles(ctx, userID) | ||
| } | ||
|
|
||
| func (q *querier) GetChatAutoArchiveDays(ctx context.Context) (int32, error) { |
There was a problem hiding this comment.
Nit [DEREM-2] Comment says "surfaced to any authenticated user via the experimental HTTP endpoint (mirroring GetChatRetentionDays)" but the handler (getChatAutoArchiveDays) enforces policy.ActionUpdate on ResourceDeploymentConfig, restricting reads to admins. getChatRetentionDays has no such check; any authenticated user can read retention days. The comment is factually inaccurate about what the HTTP layer does. (Netero)
🤖
There was a problem hiding this comment.
🤖
addressed (6bea837): Rewrote the comment. It now states accurately that dbauthz only requires an actor at this layer, and that the admin gate (policy.ActionUpdate on ResourceDeploymentConfig) lives in the HTTP handler rather than mirroring getChatRetentionDays (which has no check).
DEREM-1 (P2): the auto-archive digest body said "within 0 days, after which they will be permanently deleted" when retention_days=0, which per our docs means chats are kept indefinitely. Admins using auto_archive_days>0 with retention_days=0 would mislead every notified user. Branch the template on retention_days="0" to render the correct indefinite-retention text. Add a second golden fixture (TemplateChatAutoArchiveDigestRetentionZero) to capture that branch. DEREM-2 (Nit): GetChatAutoArchiveDays in dbauthz said it was "surfaced to any authenticated user via the experimental HTTP endpoint (mirroring GetChatRetentionDays)" but the HTTP handler is admin-only (policy.ActionUpdate on ResourceDeploymentConfig), unlike getChatRetentionDays which has no check. Correct the comment.
Relates to CODAGT-26