Skip to content

feat(db): role-keyed dbFor clients for cleanup and exec workloads#5583

Merged
TheodoreSpeaks merged 5 commits into
stagingfrom
feat/db-cleanup-exec-pools
Jul 22, 2026
Merged

feat(db): role-keyed dbFor clients for cleanup and exec workloads#5583
TheodoreSpeaks merged 5 commits into
stagingfrom
feat/db-cleanup-exec-pools

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • Adds cleanup and exec entries to DB_POOL_PROFILES and a lazy, per-role-cached dbFor('cleanup' | 'exec') factory in @sim/db — sub-process pools selected per call-site, for workloads that live inside an existing process: cleanup jobs run in the trigger.dev worker, inline execution log writes run in the Next.js web server. SIM_DB_ROLE remains restricted to web/trigger/realtime.
  • Inert until infra exists: dbFor resolves DATABASE_URL_CLEANUP / DATABASE_URL_EXEC with fallback to the base DATABASE_URL, so today these clients hit the same Postgres with their own local pool. DB_APP_NAME does not leak into them — they always report their profile appName (sim-cleanup / sim-exec). Both are tripwire-instrumented (instrumentPoolClient) with the role as the pool label.
  • Cleanup callsites: background/cleanup-logs.ts, cleanup-soft-deletes.ts, and cleanup-tasks.ts run all their queries on dbFor('cleanup'); the lib/cleanup/batch-delete.ts helpers now accept an optional dbClient (default: global db, so other consumers are unchanged). snapshotService.cleanupOrphanedSnapshots (only invoked from cleanup-logs) also moved to the cleanup pool.
  • Exec callsites: execution-log persistence in lib/logs/execution/logger.ts (idempotency read, start INSERT, the self-contained completion transaction, getWorkflowExecution), all workflow_execution_logs writes in logging-session.ts (progress-marker fallback UPDATEs, status reads, markExecutionAsFailed), and the workflow-state snapshot upsert in snapshot/service.ts run on dbFor('exec'). Billing/usage-ledger work (advisory-lock reconcile transaction, subscription/org lookups, PII-settings read) deliberately stays on the default client.
  • Test infra: databaseMock / dbChainMock in @sim/testing expose dbFor returning the shared mock instance; local @sim/db mocks in the touched suites updated the same way.

Type of Change

  • New feature

Testing

  • packages/db: tsc --noEmit clean; vitest 24/24 (tx-tripwire + connection-url).
  • apps/sim: tsc --noEmit — no new errors (one pre-existing error in providers/meta/index.ts exists on clean origin/staging).
  • vitest: cleanup suites, lib/logs/execution/** (191 tests), plus every other suite that locally mocks @sim/db and transitively imports the touched modules (66 tests) — all passing.
  • bun run lint:check and bun run check:api-validation:strict pass.

Checklist

  • I have signed the CLA

🤖 Generated with Claude Code

https://claude.ai/code/session_01NNy9Fzpfc1FdHAzYyb6Ycy

@vercel

vercel Bot commented Jul 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Jul 22, 2026 6:10pm

Request Review

@cursor

cursor Bot commented Jul 11, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches high-volume execution logging, pause/resume, and retention cleanup; mis-pooled connections could affect timeouts or billing if URLs diverge, but fallbacks keep current behavior until dedicated URLs are set.

Overview
Introduces dbFor('cleanup' | 'exec') in @sim/db: lazy, cached Drizzle clients with their own pool profiles (sim-cleanup / sim-exec), optional DATABASE_URL_CLEANUP / DATABASE_URL_EXEC (fallback to the process URL so behavior is unchanged until infra is wired), and SIM_DB_ROLE limited to web / trigger / realtime.

Cleanup path — background jobs (cleanup-logs, cleanup-soft-deletes, cleanup-tasks), chat cleanup reads, large-value metadata pruning during cleanup, and orphaned snapshot deletion route through dbFor('cleanup'). Shared batch-delete helpers accept optional dbClient; billing-coupled file deletes in soft-deletes still use the default db.

Exec path — workflow execution log persistence, logging-session writes, snapshot upserts/reads, large-value registration/reference writes, and human-in-the-loop DB work use dbFor('exec'). Usage/billing reconcile and related ledger reads stay on the global client.

Tests and @sim/testing mocks expose dbFor returning the same mock instance as db.

Reviewed by Cursor Bugbot for commit d106359. Bugbot is set up for automated code reviews on this repo. Configure here.

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds role-keyed database clients for cleanup and execution workloads. The main changes are:

  • New cleanup and exec pool profiles in @sim/db.
  • A cached dbFor() factory for per-workload database clients.
  • Cleanup jobs routed through the cleanup client.
  • Execution log, snapshot, pause, and resume persistence routed through the exec client.
  • Test mocks updated to expose dbFor().

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The latest cleanup fixes keep the affected cleanup reads and writes on the cleanup client.
  • Snapshot reads and writes now share the exec client.
  • The remaining default-client billing work stays tied to the billing transaction.

Important Files Changed

Filename Overview
packages/db/db.ts Adds cleanup and exec pool profiles and a cached dbFor() factory while keeping process role selection limited to process roles.
apps/sim/background/cleanup-logs.ts Routes cleanup log queries, large-value tombstoning, metadata pruning, and batch deletes through the cleanup client.
apps/sim/background/cleanup-soft-deletes.ts Routes soft-delete cleanup reads and deletes through the cleanup client while keeping billable-file billing work on the default client.
apps/sim/background/cleanup-tasks.ts Routes task cleanup queries and delete helper calls through the cleanup client.
apps/sim/lib/cleanup/batch-delete.ts Adds optional database client parameters to shared batch-delete helpers.
apps/sim/lib/cleanup/chat-cleanup.ts Moves chat file collection and survivor checks onto the cleanup client.
apps/sim/lib/execution/payloads/large-value-metadata.ts Moves execution-time large-value metadata writes to the exec client and allows cleanup helpers to receive an explicit client.
apps/sim/lib/logs/execution/logger.ts Routes execution log reads, inserts, completion transactions, and lookups through the exec client.
apps/sim/lib/logs/execution/logging-session.ts Routes progress marker writes, status reads, and failure updates through the exec client.
apps/sim/lib/logs/execution/snapshot/service.ts Routes snapshot upserts and reads through the exec client and orphan cleanup through the cleanup client.
apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts Routes paused execution, resume queue, and related execution-log persistence through the exec client.
packages/testing/src/mocks/database.mock.ts Adds dbFor() to shared database mocks so role-keyed clients use the existing mock instance.

Reviews (8): Last reviewed commit: "feat(db): route pause/resume and large-v..." | Re-trigger Greptile

Comment thread apps/sim/lib/logs/execution/snapshot/service.ts
Comment thread apps/sim/lib/logs/execution/snapshot/service.ts
Comment thread apps/sim/lib/logs/execution/logger.ts
Comment thread apps/sim/background/cleanup-logs.ts
Comment thread apps/sim/background/cleanup-tasks.ts
@greptile-apps

greptile-apps Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds workload-specific database clients for cleanup and execution logging. The main changes are:

  • Adds cleanup and exec pool profiles plus a cached dbFor factory.
  • Routes cleanup jobs through the cleanup client.
  • Routes execution-log persistence and snapshot writes through the exec client.
  • Lets batch-delete helpers accept an explicit database client.
  • Updates env validation and database mocks for the new keyed clients.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The changed cleanup paths consistently use the cleanup client.
  • The changed execution-log paths consistently use the exec client where intended.
  • The new database factory is cached per role and keeps process-role validation separate from workload clients.

Important Files Changed

Filename Overview
packages/db/db.ts Adds cleanup and exec pool profiles, keeps process roles restricted, and exposes a cached dbFor factory.
apps/sim/lib/cleanup/batch-delete.ts Adds an optional client parameter to cleanup delete helpers while preserving the existing default client.
apps/sim/background/cleanup-logs.ts Moves cleanup log queries and delete helpers to the cleanup client.
apps/sim/background/cleanup-soft-deletes.ts Moves soft-delete cleanup selects and deletes to the cleanup client.
apps/sim/background/cleanup-tasks.ts Moves task cleanup queries and deletes to the cleanup client.
apps/sim/lib/logs/execution/logger.ts Moves execution-log persistence to the exec client while keeping billing work on the default client.
apps/sim/lib/logs/execution/logging-session.ts Moves progress markers, status reads, and failure writes to the exec client.
apps/sim/lib/logs/execution/snapshot/service.ts Moves snapshot upserts to the exec client and orphan cleanup to the cleanup client.
apps/sim/lib/core/config/env.ts Adds optional keyed database URLs for cleanup and exec pools.
packages/testing/src/mocks/database.mock.ts Extends shared database mocks so dbFor returns the same mock instance as db.

Reviews (2): Last reviewed commit: "feat(db): role-keyed dbFor clients for c..." | Re-trigger Greptile

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c7ac96b. Configure here.

Comment thread packages/db/db.ts
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

TheodoreSpeaks and others added 5 commits July 22, 2026 11:02
…e pools

Route markLargeValuesDeleted / pruneLargeValueMetadata (optional dbClient) and
chat-cleanup's file collection through the cleanup pool, and getSnapshot through
the exec pool, so the cleanup and inline-execution workloads stop borrowing the
process-wide pool for these queries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNy9Fzpfc1FdHAzYyb6Ycy
With DATABASE_URL_WEB/TRIGGER set (as in prod) and the sub-pool URLs unset,
falling back to the base URL would silently shift execution-log and cleanup
traffic to a different PgBouncer endpoint on deploy. Chain the fallback
through the URL the process itself resolved so the rollout stays inert.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNy9Fzpfc1FdHAzYyb6Ycy
One line per role at first use: the dedicated DATABASE_URL_<ROLE> when set,
otherwise an explicit fallback message naming the process connection it
shares — so a missing/typo'd env var is visible at rollout instead of silent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNy9Fzpfc1FdHAzYyb6Ycy
…the exec pool

Coverage scan follow-up: the paused_executions / resume_queue /
workflow_execution_logs transactions in human-in-the-loop-manager.ts and the
large-value owner/reference registration writes on the execution path now use
dbFor('exec'), matching the completion writes in the execution logger. All are
self-contained; billing calls remain outside the moved transactions on the
default client.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NNy9Fzpfc1FdHAzYyb6Ycy
@TheodoreSpeaks
TheodoreSpeaks force-pushed the feat/db-cleanup-exec-pools branch from fe5a9ce to d106359 Compare July 22, 2026 18:05
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@TheodoreSpeaks
TheodoreSpeaks merged commit 70814bc into staging Jul 22, 2026
20 checks passed
@TheodoreSpeaks
TheodoreSpeaks deleted the feat/db-cleanup-exec-pools branch July 22, 2026 18:35
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