Skip to content

[Improve] Harden schema: host-aware uniqueness, composite webhook keys, retention, and classification constraints - #136

Merged
mrubens merged 1 commit into
developfrom
schema-hardening-constraints
Jul 10, 2026
Merged

[Improve] Harden schema: host-aware uniqueness, composite webhook keys, retention, and classification constraints#136
mrubens merged 1 commit into
developfrom
schema-hardening-constraints

Conversation

@mrubens

@mrubens mrubens commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Finishes the schema-hardening leftovers from finding 9 of the pre-release architecture audit (P1). Follows up on #100, #102, and #103.

What changed

1. Host-aware repository uniqueness

repositories was unique on (source_control_provider, full_name) and (source_control_provider, external_repo_id), so identical names/ids on different self-managed GitLab/Gitea/ADO hosts collided. Both unique indexes now include the host:

  • repositories_provider_host_full_name_unique on (provider, coalesce(host, ''), full_name)
  • repositories_provider_host_external_repo_unique on (provider, coalesce(host, ''), external_repo_id)

host is nullable until historical rows are backfilled, so NULLs are coalesced to '' inside the index. That keeps un-backfilled rows colliding with each other exactly as strictly as before (a plain column index would treat NULLs as distinct and silently weaken the constraint), which also means the new indexes are strictly weaker than the old ones and cannot fail on existing data. All four production sync paths already stamp a non-null normalized host (github.com literal; new URL(baseUrl).host for GitLab/Gitea/ADO); the repository factory now stamps host too so tests exercise the same shape. No onConflict anywhere targets these indexes (verified: all repo upserts are find-then-update).

2. Composite webhook delivery key

webhooks.delivery_id was globally unique; delivery ids are only unique per provider (GitHub GUIDs, Linear delivery ids, sha256-of-body fallbacks for GitLab/Gitea/ADO). Now unique on (provider, delivery_id). Both recordWebhook helpers dedupe via untargeted ON CONFLICT DO NOTHING, so no code change was needed there; the redundant webhooks_provider_idx is dropped because the new unique index leads with provider.

3. Webhook payload redaction + retention

  • Redaction at write time: apps/api/src/handlers/webhook-payload-redaction.ts deep-masks clearly credential-bearing keys (secret, token, password, credentials, authorization, *_secret, *_token, api_key, private_key, ... in snake/camel/kebab case) before the payload row is stored. Handlers still receive the original payload; only the stored audit copy is redacted. Free-text fields (comment bodies, PR descriptions) are intentionally kept.
  • Retention: new daily WebhookCleanup job on the existing scheduled-jobs BullMQ queue deletes rows older than WEBHOOK_RETENTION_DAYS (new env var, default 30). The batched delete lives in packages/db/src/lib/webhook-retention.ts (deleteExpiredWebhooks) and is covered by a real-database test; the first run on a long-lived deployment drains the backlog in bounded batches.

4. Database-level CHECKs for durable task classifications

Following the initiator-stamp CHECK pattern:

  • tasks: workflow, surface, trigger, visibility, state, harness, requested_work_kind, requested_work_kind_source, commit_author_kind (NULL-tolerant)
  • task_runs: kind, harness

The vocabulary source of truth stays in @roomote/types; a real-database test (schema-constraints.test.ts) iterates every exported constant value and asserts Postgres accepts it, so extending a vocabulary without updating the CHECK (schema + migration) fails CI. run status was deliberately left unconstrained (mutable runtime lifecycle, not a durable classification).

5. tasks.provider renamed to tasks.model_provider

Confirmed ambiguous: it is the inference-provider slot paired with tasks.model (only production writer stamps the 'opencode' constant; only reader is the instance telemetry report), while source-control/communication/compute providers all have their own typed columns. Renamed column + Drizzle property (modelProvider), plus DEFAULT_STANDARD_TASK_PROVIDER -> DEFAULT_STANDARD_TASK_MODEL_PROVIDER. Not exposed in any API/frontend surface.

Migration

0006_schema_hardening (+ snapshot, journal). Verified three ways against real Postgres 17:

  1. Full chain 0000 -> 0006 on a fresh database.
  2. Upgrade simulation: chain to 0005, seed representative rows (tasks/runs/webhooks/NULL-host repository), then apply 0006 -- rename preserves data and all constraint adds validate.
  3. db:push:test parity for the test suite.

Validation

  • packages/db: full suite (238 tests) including new real-DB schema-constraints.test.ts and webhook-retention.test.ts
  • apps/api: all webhook handler suites (223 tests) including new redaction coverage
  • apps/bullmq (91), packages/cloud-agents (433), packages/env (65), targeted packages/sdk title-refresh
  • pnpm lint:fast, pnpm check-types:fast clean. pnpm knip verified clean via a baseline diff and --no-gitignore (knip misbehaves inside .agents/worktrees/ checkouts because the parent repo gitignores that path; it passes in a normal checkout).

Notes for reviewers

  • A parallel agent-identity PR may claim migration number 0006; whoever merges second rebases and renumbers.
  • Known limitation carried over unchanged: the GitHub sync hardcodes host: 'github.com' (no GHES support today), so host-aware uniqueness currently only has teeth for GitLab/Gitea/ADO.

…s, retention, and classification constraints

- repositories: scope uniqueness to (provider, host, fullName/externalRepoId)
  with NULL hosts coalesced to '' so un-backfilled rows keep colliding
- webhooks: delivery ids are now unique per (provider, delivery_id) instead
  of globally; drop the redundant provider index
- webhooks: redact clearly sensitive payload fields before storage and add a
  daily WebhookCleanup BullMQ job honoring WEBHOOK_RETENTION_DAYS (default 30)
- tasks/task_runs: CHECK constraints for workflow, surface, trigger,
  visibility, state, harness, requested-work, commit-author kind, run kind
- tasks: rename ambiguous provider column to model_provider
- migration 0006 + real-database tests covering every vocabulary value and
  the new uniqueness semantics
@roomote-roomote

roomote-roomote Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

No code issues found. See task

Reviewed the schema-hardening changes: host-aware repository uniqueness (NULL-host coalesce to ''), the composite (provider, delivery_id) webhook key, write-time payload redaction, the daily WebhookCleanup retention job, the tasks/task_runs classification CHECKs, and the tasks.provider -> tasks.model_provider rename.

Notably solid: all webhook insert paths (GitHub/GitLab/Gitea/ADO share recordWebhook; Linear has its own) run through redactWebhookPayload, so redaction coverage is complete; the rename is fully propagated (no lingering DEFAULT_STANDARD_TASK_PROVIDER or tasks.provider readers/writers); the batched delete is bounded and the webhooks_created_at_idx supports its filter; and schema-constraints.test.ts iterates the @roomote/types vocabularies against real Postgres so a CHECK cannot silently drift from the constants.

One deployment-time caveat (not a code defect): the tasks/task_runs ADD CONSTRAINT ... CHECK statements validate all existing rows, so migration 0006 will abort if any legacy row holds an out-of-vocabulary value. This matches the existing initiator-stamp CHECK pattern and the PR notes it was verified against representative data.

@mrubens
mrubens merged commit 3066059 into develop Jul 10, 2026
1 check passed
@mrubens
mrubens deleted the schema-hardening-constraints branch July 10, 2026 22:56
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.

2 participants