Skip to content

feat: auto-archive chats#24486

Draft
johnstcn wants to merge 39 commits intomainfrom
cj/chat-auto-archive
Draft

feat: auto-archive chats#24486
johnstcn wants to merge 39 commits intomainfrom
cj/chat-auto-archive

Conversation

@johnstcn
Copy link
Copy Markdown
Member

@johnstcn johnstcn commented Apr 17, 2026

  • Adds dbpurge support for auto-archiving chats
  • Adds notification template for chat auto-archival

Relates to CODAGT-26

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.
@johnstcn johnstcn self-assigned this Apr 17, 2026
@johnstcn johnstcn force-pushed the cj/chat-auto-archive branch from 6018381 to ad22b73 Compare April 17, 2026 15:55
Comment thread docs/admin/integrations/prometheus.md Outdated
Comment thread coderd/database/dbauthz/dbauthz.go Outdated
Copy link
Copy Markdown
Member Author

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self-reviewed

@johnstcn johnstcn force-pushed the cj/chat-auto-archive branch from 9fd96ff to e4a0406 Compare April 20, 2026 15:39
@johnstcn johnstcn force-pushed the cj/chat-auto-archive branch from e4a0406 to a18186c Compare April 20, 2026 16:36
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.
johnstcn added 16 commits April 20, 2026 20:16
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.
@johnstcn johnstcn force-pushed the cj/chat-auto-archive branch from a18186c to 015ee07 Compare April 20, 2026 19:16
Base automatically changed from cj/codagt-200 to main April 21, 2026 10:11
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.
@johnstcn
Copy link
Copy Markdown
Member Author

/coder-agents-review

Copy link
Copy Markdown

@coder-agents-review coder-agents-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

🤖

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖

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) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

🤖

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖

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.
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.

1 participant