[jzhu13]

Problem

• An insight alert with the investigation agent on holds its notification until the agent's verdict, and a safety net sends it if the agent stalls.
• The shared alerts platform delivers every evaluation as soon as it records it, so these alerts cannot move to it yet.
• This adds a generic hold to the platform. No source sets one yet; the insight adapter does in a later layer.

Changes

• A source can put an evaluation's delivery on PlatformAlertOutcome.held_delivery (request, reason, deadline) instead of starting it.
• record_outcomes writes the hold to a new PlatformAlertDeliveryHold table in the outcome's own transaction. A recorded firing cannot lose its hold.
• release_hold in the facade releases a hold once, with send or suppress:
  • The first release wins under a row lock.
  • Only a send returns the request, and only to the winning releaser or its retry. The caller starts the delivery with start_deliveries, so the delivery keeps its usual id.
• reschedule_hold moves an unreleased hold's deadline, for a source that needs a shorter grace once it knows the outcome.
• deliver_evaluation refuses an evaluation whose hold is unreleased or suppressed, as a second guard.
• Safety net: alerts-platform-release-expired-holds runs every minute on the delivery queue.
  • It sends up to 200 holds past their deadline, within one activity payload.
  • It re-reads each deadline under the row lock, so a hold rescheduled after the sweep found it stays held.
  • It suppresses the hold of a disabled alert, so turning the alert back on never sends an old firing.
  • It still sends holds of an alert rolled back to its source, because the request records the owner at check time.
  • It starts any released send that nobody marked started 5 minutes after the release. A releaser that fails between the release and the start still delivers.
  • It prunes holds older than the 90-day history TTL.
• record_hold_delivery_started lets a releaser mark the delivery it started, so the sweep does not start it again.

Warning

A disabled alert's held firing is suppressed, not sent on re-enable. This departs on purpose from the insight stack's own safety net, which skips a disabled alert and sends the firing whenever the alert comes back.

• New metric alerts_platform_holds_expired_total counts safety-net sends. A rise means a source holds deliveries and does not release them.
• docs/internal/alerts-platform-temporal.md gets a "Held deliveries" section.
• Nothing user-visible changes until a source holds a delivery.

Note

A worker on the previous release decodes PlatformAlertOutcome without held_delivery. The source change that sets it must ship at least one deploy after this one.

How did you test this code?

Test rationale:

• test_delivery_holds.py (new, no existing coverage for holds):
  • The hold is written with its outcome, and a replayed record adds none.
  • One release wins, and only its send returns the request, including a retry from the same releaser.
  • The sweep sends holds past their deadline and released sends nobody started. It keeps a hold rescheduled after the find, suppresses a disabled alert's hold, and prunes old ones.
  • One sweep returns no more requests than fit its payload budget.
• test_delivery_evaluation.py: one parameterized case shows an unreleased or suppressed hold sends nothing and a released send goes out. Its default configuration id became a UUID, because the guard queries a UUID column.
• Ran locally: the new tests, test_delivery_evaluation.py, test_temporal_workflows.py, test_temporal_schedule.py, and the full products/alerts_platform suite. 17 platform tests that read history from ClickHouse fail locally because the local test schema predates the source_detail column on master. CI runs them against a fresh schema.
• Not run: the sweep workflow end to end against a Temporal test server.

Release status

• No feature flag controls this change

Docs update

Updated docs/internal/alerts-platform-temporal.md.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

• Layer I2 of the plan for moving investigating insight alerts onto the alerts platform. It is independent of the investigation-store layer (I3).
• Skills: /django-migrations, /writing-tests, /code-review high --fix, /simplify, /reviewing-with-coderabbit, /writing-pr-descriptions.
• /code-review split pruning out of the expiry lookup. /simplify moved from_stored onto Temporal's own converter, tied hold retention to the history TTL, and kept a failed prune from blocking the sweep.
• PostHog Review findings, all fixed: the safety net now re-reads the deadline under the lock, recovers a send whose start was lost, and bounds its result by payload size.
• Suppressing a disabled alert's hold, instead of skipping it, is the owner's decision.
• CodeRabbit CLI, --deep: one finding, fixed. A hold reason longer than its column, or a negative deadline, is now rejected when the hold is built instead of failing the batch record.
• No duplicate: an open-PR search for delivery holds found nothing.

[trunk-io]

😎 Merged successfully - details.

[github-actions]

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Comment density — 4% of added code lines are comments (27 of 673)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File	Comment lines	Added lines
products/alerts_platform/backend/logic/delivery_holds.py	6	207
products/alerts_platform/backend/models/platform_alert.py	6	48
products/alerts_platform/backend/temporal/workflows.py	6	93
products/alerts_platform/backend/facade/contracts.py	5	41
products/alerts_platform/backend/logic/platform_lifecycle.py	2	15
products/alerts_platform/backend/tests/test_delivery_holds.py	2	169

This check does not block merging. It updates on every push and clears when the share drops.

⚠️ Django migration SQL — 1 new migration to review

We've detected new migrations on this PR. Review the SQL output for each migration:

products/alerts_platform/backend/migrations/0020_delivery_hold.py

/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
  return result
2026-10-26T21:17:15.286654Z [error    ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=6526 tid=140242322107264
Traceback (most recent call last):
  File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
    geoip: Optional[GeoIP2] = GeoIP2(cache=8)
                              ~~~~~~^^^^^^^^^
  File "/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
    raise GeoIP2Exception(
        "Path must be a valid database or directory containing databases."
    )
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
  return (ssh_host,
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
  from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:

WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model PlatformAlertDeliveryHold
--
CREATE TABLE "alerts_platformalertdeliveryhold" ("id" uuid NOT NULL PRIMARY KEY, "evaluation_key" varchar(255) NOT NULL, "reason" varchar(64) NOT NULL, "request" jsonb NOT NULL, "held_at" timestamp with time zone NOT NULL, "release_after" timestamp with time zone NOT NULL, "released_at" timestamp with time zone NULL, "release_action" varchar(16) NULL, "released_by" varchar(16) NULL, "delivery_started_at" timestamp with time zone NULL, "configuration_id" uuid NOT NULL, "team_id" integer NOT NULL, CONSTRAINT "platform_alert_delivery_hold_one_per_evaluation" UNIQUE ("configuration_id", "evaluation_key"));
ALTER TABLE "alerts_platformalertdeliveryhold" ADD CONSTRAINT "alerts_platformalert_configuration_id_376c2fa3_fk_alerts_pl" FOREIGN KEY ("configuration_id") REFERENCES "alerts_platformalertconfiguration" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "alerts_platformalertdeliveryhold_configuration_id_376c2fa3" ON "alerts_platformalertdeliveryhold" ("configuration_id");
CREATE INDEX "alerts_platformalertdeliveryhold_team_id_19cc58b1" ON "alerts_platformalertdeliveryhold" ("team_id");
CREATE INDEX "platform_alert_hold_due_idx" ON "alerts_platformalertdeliveryhold" ("release_after") WHERE "released_at" IS NULL;
CREATE INDEX "platform_alert_hold_start_idx" ON "alerts_platformalertdeliveryhold" ("released_at") WHERE ("delivery_started_at" IS NULL AND "release_action" = 'send');
CREATE INDEX "platform_alert_hold_age_idx" ON "alerts_platformalertdeliveryhold" ("held_at");
COMMIT;

Last updated: 2026-10-26 21:18 UTC (f7919aa)

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 1 Safe | 0 Needs Review | 0 Blocked

✅ Safe

Brief or no lock, backwards compatible

alerts_platform.0020_delivery_hold
  └─ #1 ✅ CreateModel
     Creating new table is safe
     model: PlatformAlertDeliveryHold
  │
  └──> ℹ️  INFO:
       ℹ️  Skipped operations on newly created tables (empty tables
       don't cause lock contention).

Last updated: 2026-10-26 21:18 UTC (f7919aa)

[posthog]

PostHog Review · Standard · reviewed 806a145: 3 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.

[trunk-io]

   

View Full Report ↗︎ ⋅ Docs

[parameterai]

Risk: No findings

This increment refines the delivery-hold safety net: the sweep now skips rows locked by other transactions, reuses delivery workflow ids with ALLOW_DUPLICATE_FAILED_ONLY so a completed delivery is never re-run, and the expired-hold metric counts only fresh deadline releases. I found no new security issues; the reuse-policy change in particular tightens double-send behavior rather than loosening it.

_{Sentinel reviewed 1f2b162 · Review settings}

[posthog]

PostHog Review · Standard · reviewed 9b09f78: 3 Should fix.

[stamphog]

Approved.

The risky changes have owning-team and STRONG-familiarity authorship assurance, with targeted tests. No specific unresolved review concerns or policy refusal grounds are evidenced; the listed formal reviews are on older commits.

• Author wrote 75% of the modified lines and has 29 merged PRs in these paths (familiarity STRONG).

Gate mechanics and policy version

Gate		Result
prerequisites	✓	all clear
deny-list	✓	no deny categories matched
size	✓	673L, 15F substantive, 931L/19F incl. docs/generated/snapshots — within ceiling
tier	✓	T1-agent / T1d-complex (931L, 19F, cross-cutting, feat)
stamphog 2.6.1		.stamphog/policy.yml @ f7919aa · reviewed head f7919aa

[jzhu13]

/trunk merge

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-10-26 23:42 UTC	Run
prod-us	✅ Deployed	2026-10-26 23:57 UTC	Run
prod-eu	✅ Deployed	2026-10-26 23:57 UTC	Run