Skip to content

chore(workflows): seal reverse accessors on team and user - #108408

Open
mayteio wants to merge 5 commits into
posthog/workflows-facade-provider-contractsfrom
posthog/workflows-seal-reverse-accessors
Open

mayteio wants to merge 5 commits into
posthog/workflows-facade-provider-contractsfrom
posthog/workflows-seal-reverse-accessors

Conversation

@mayteio

@mayteio mayteio commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Refs #84402

Changes

  • The 12 workflows relations to Team and User now use related_name="+". Django no longer creates the reverse accessors or the reverse query names.
  • Migration workflows.0027 records the change as 12 AlterField operations. sqlmigrate shows each one as a no-op, because related_name is a Python-only attribute.
  • The crossings baseline loses its 12 workflows reverse-accessor rows, so a new unsealed relation fails CI.
  • WorkflowProposal.resolved_by loses its explicit name resolved_workflow_proposals. No code uses it.
  • Nothing changes for users. Cascade deletes from Team and User still work, because Django keeps the hidden relations for deletion.

Note

The migration risk analyzer marks this migration "Needs Review", because each AlterField is on a foreign key. It generates no SQL, so it takes no lock on posthog_team or posthog_user. Earlier sealing migrations in other products have the same shape, for example web_analytics.0012.

How did you test this code?

  • makemigrations --check reports no changes.

  • sqlmigrate workflows 0027 shows a no-op for all 12 operations.

  • The migration applied to the dev database and to the prewarmed test_posthog database before the test run.

  • A search of all non-migration Python found no use of the removed accessors or of their __ query forms.

  • Tests run against the migrated database: products/workflows, the team and user deletion tests in posthog/api/test/test_team.py, test_user.py and posthog/models/test/test_team_model.py, and the hogli product check tests. hogli product:lint workflows also passes.

  • Manual checks on the local dev stack, with CDP, Temporal and a Temporal worker running, and no mocks:

    Check Branch Master
    Browser: log in, create and edit a workflow, workflow tabs, library, channels, opt-outs, reputation, admin pages for Team and User Pass Not run
    API: workflows, templates, schedules, batch jobs with the real CDP call, revisions and restore, proposals reject and approve Pass Not run
    Django reverse fields from workflows on Team and User None 12
    Member leaves the organization, then deletes the account through the API. created_by and resolved_by become null Pass Pass
    Project delete through the API and the Temporal delete workflow. All workflows rows and the team go Pass Pass
    The two delete rows above, when a HogFlowBatchJob exists Fail Fail
  • Not run: repo-wide mypy and the full backend suite. CI runs both.

Note

The manual checks found a separate bug that is also on master. Both foreign keys on HogFlowBatchJob use on_delete=DO_NOTHING. An account delete for a user who created a batch job returns a 500 IntegrityError. A project delete for a project with a batch job makes the Temporal delete workflow retry without end. This PR does not change on_delete, and master fails in the same way.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: PostHog Desktop (Claude Code), Claude Opus 5.5 (claude-opus-5-5)

  • Skills invoked: /django-migrations, /writing-pr-descriptions.
  • No duplicate: no open PR changes these relations. W-PR1 (chore(workflows): add facade and route callers through it #107597) edits team_workflows_config.py on other lines and adds no workflows migration, so the two do not conflict.
  • This PR is based on master and does not depend on W-PR1.
  • The master column in the manual checks comes from the same scripts, run with the master model files in place.

Created with PostHog Desktop

🤖 Generated with Claude Code

Set related_name="+" on the 12 workflows relations to Team and User, with a state-only AlterField migration, and remove the 12 reverse-accessor rows from the crossings baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: 90c83620-aeca-40af-ad31-307e8edf458b
@mayteio mayteio self-assigned this Sep 29, 2026
@trunk-io

trunk-io Bot commented Sep 29, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 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) — 1 new duplicated block (worst 105 tokens)

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.

First copy Second copy Lines Tokens
products/workflows/backend/models/hog_flow/hog_flow.py:150 products/workflows/backend/models/hog_flow/hog_flow_template.py:45 14 105
✅ 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.

✅ Playwright — all passed

All tests passed.

View test results →

⚠️ Backend coverage — 96.0% of changed backend lines covered — 2 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ███████████████████░ 96.0% (54 / 56)

File Patch Uncovered changed lines
products/workflows/backend/providers/twilio.py 80.0% 51
products/workflows/backend/facade/api.py 83.3% 287

🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 538501421566200 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)
Product Coverage Lines
warehouse_sources_queue ██░░░░░░░░░░░░░░░░░░ 10.5% 187 / 1,777
platform_features ██░░░░░░░░░░░░░░░░░░ 12.1% 7 / 58
demo ███████████░░░░░░░░░ 52.8% 1,411 / 2,673
data_tools ████████████░░░░░░░░ 61.2% 90 / 147
ai_gateway ███████████████░░░░░ 75.0% 9 / 12
aeo ███████████████░░░░░ 76.3% 617 / 809
batch_exports ████████████████░░░░ 81.2% 21,528 / 26,502
apm █████████████████░░░ 84.1% 1,306 / 1,553
cdp ██████████████████░░ 88.2% 4,545 / 5,155
ml_inference ██████████████████░░ 88.4% 509 / 576
mcp_analytics ██████████████████░░ 89.2% 5,038 / 5,651
product_tours ██████████████████░░ 89.3% 1,331 / 1,491
dashboards ██████████████████░░ 89.5% 6,904 / 7,714
data_warehouse ██████████████████░░ 90.0% 14,027 / 15,589
notebooks ██████████████████░░ 90.2% 15,297 / 16,964
signals ██████████████████░░ 90.2% 57,836 / 64,097
cohorts ██████████████████░░ 90.4% 8,420 / 9,316
streamlit_apps ██████████████████░░ 90.8% 2,684 / 2,956
managed_warehouse ██████████████████░░ 91.0% 10,252 / 11,263
tasks ██████████████████░░ 91.1% 75,525 / 82,874
data_modeling ██████████████████░░ 91.4% 10,554 / 11,543
exports ██████████████████░░ 91.6% 9,680 / 10,562
engineering_analytics ██████████████████░░ 91.7% 11,032 / 12,030
ai_training ██████████████████░░ 92.2% 356 / 386
business_knowledge ██████████████████░░ 92.2% 7,684 / 8,330
conversations ███████████████████░ 92.5% 28,734 / 31,062
early_access_features ███████████████████░ 92.6% 1,332 / 1,439
managed_migrations ███████████████████░ 92.7% 1,581 / 1,705
visual_review ███████████████████░ 92.8% 9,247 / 9,969
stamphog ███████████████████░ 92.8% 8,109 / 8,742
canvas ███████████████████░ 92.8% 6,873 / 7,405
approvals ███████████████████░ 93.0% 3,974 / 4,271
mcp_registry ███████████████████░ 93.1% 1,670 / 1,794
notifications ███████████████████░ 93.2% 1,145 / 1,229
error_tracking ███████████████████░ 93.2% 16,359 / 17,547
surveys ███████████████████░ 93.3% 6,571 / 7,040
slack_app ███████████████████░ 93.4% 13,995 / 14,989
autoresearch ███████████████████░ 93.6% 8,481 / 9,061
context_layer ███████████████████░ 93.9% 3,415 / 3,638
web_analytics ███████████████████░ 94.0% 21,815 / 23,218
alerts ███████████████████░ 94.0% 8,570 / 9,114
billing_alerts ███████████████████░ 94.1% 2,094 / 2,226
mcp_store ███████████████████░ 94.4% 8,940 / 9,472
ai_observability ███████████████████░ 94.5% 24,473 / 25,896
workflows ███████████████████░ 94.6% 15,249 / 16,120
wizard ███████████████████░ 94.7% 6,150 / 6,496
reminders ███████████████████░ 94.8% 760 / 802
review_hog ███████████████████░ 95.0% 11,507 / 12,119
endpoints ███████████████████░ 95.1% 9,206 / 9,681
annotations ███████████████████░ 95.1% 817 / 859
customer_analytics ███████████████████░ 95.2% 25,636 / 26,938
legal_documents ███████████████████░ 95.2% 2,311 / 2,427
marketing_analytics ███████████████████░ 95.3% 19,566 / 20,528
posthog_ai ███████████████████░ 95.3% 2,488 / 2,610
experiments ███████████████████░ 95.4% 32,457 / 34,020
actions ███████████████████░ 95.5% 756 / 792
logs ███████████████████░ 95.5% 15,399 / 16,130
data_catalog ███████████████████░ 95.5% 4,401 / 4,606
tracing ███████████████████░ 95.6% 3,536 / 3,699
replay_vision ███████████████████░ 95.6% 27,675 / 28,939
growth ███████████████████░ 95.7% 11,228 / 11,734
messaging ███████████████████░ 95.8% 3,798 / 3,963
skills ███████████████████░ 95.8% 6,972 / 7,274
product_analytics ███████████████████░ 96.0% 28,470 / 29,647
access_control ███████████████████░ 96.3% 7,113 / 7,386
revenue_analytics ███████████████████░ 96.4% 1,876 / 1,946
user_interviews ███████████████████░ 96.5% 2,867 / 2,971
feature_flags ███████████████████░ 96.5% 25,588 / 26,509
warehouse_sources ███████████████████░ 97.3% 458,451 / 471,303
data_quality ████████████████████ 97.6% 7,587 / 7,774
links ████████████████████ 97.9% 234 / 239
security ████████████████████ 98.0% 1,286 / 1,312
metrics ████████████████████ 98.0% 4,084 / 4,166
analytics_platform ████████████████████ 98.3% 2,784 / 2,833
pulse ████████████████████ 98.5% 2,046 / 2,078
live_debugger ████████████████████ 99.2% 626 / 631
field_notes ████████████████████ 99.4% 172 / 173

Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.

⚠️ Django migration SQL — 1 new migration to review

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

products/workflows/backend/migrations/0027_seal_reverse_accessors.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
/home/runner/work/tmp/tool_cache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
  ed = p(logger, meth_name, ed)  # type: ignore[arg-type]
2026-09-29T18:47:48.241841Z [error    ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=11345 tid=140417331678080
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;
--
-- Alter field created_by on hogflow
--
-- (no-op)
--
-- Alter field team on hogflow
--
-- (no-op)
--
-- Alter field created_by on hogflowbatchjob
--
-- (no-op)
--
-- Alter field team on hogflowbatchjob
--
-- (no-op)
--
-- Alter field created_by on hogflowrevision
--
-- (no-op)
--
-- Alter field team on hogflowrevision
--
-- (no-op)
--
-- Alter field team on hogflowschedule
--
-- (no-op)
--
-- Alter field created_by on hogflowtemplate
--
-- (no-op)
--
-- Alter field team on hogflowtemplate
--
-- (no-op)
--
-- Alter field team on teamworkflowsconfig
--
-- (no-op)
--
-- Alter field resolved_by on workflowproposal
--
-- (no-op)
--
-- Alter field team on workflowproposal
--
-- (no-op)
COMMIT;

Last updated: 2026-09-29 18:48 UTC (da807bc)

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 0 Safe | 1 Needs Review | 0 Blocked

⚠️ Needs Review

May have performance impact

workflows.0027_seal_reverse_accessors
  └─ #1 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflow, field: created_by, field_type: ForeignKey
  └─ #2 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflow, field: team, field_type: ForeignKey
  └─ #3 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowbatchjob, field: created_by, field_type: ForeignKey
  └─ #4 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowbatchjob, field: team, field_type: ForeignKey
  └─ #5 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowrevision, field: created_by, field_type: ForeignKey
  └─ #6 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowrevision, field: team, field_type: ForeignKey
  └─ #7 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowschedule, field: team, field_type: ForeignKey
  └─ #8 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowtemplate, field: created_by, field_type: ForeignKey
  └─ #9 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: hogflowtemplate, field: team, field_type: ForeignKey
  └─ #10 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: teamworkflowsconfig, field: team, field_type: OneToOneField
  └─ #11 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: workflowproposal, field: resolved_by, field_type: ForeignKey
  └─ #12 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: workflowproposal, field: team, field_type: ForeignKey

Last updated: 2026-09-29 18:49 UTC (da807bc)

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Critical risk] Database schema changes to seal reverse accessors on workflow models.

The PR appears safe to merge.

Reviews (2) · Last reviewed commit: "chore(workflows): drop stale resolved_wo..."

Comment thread products/workflows/backend/models/workflow_proposal.py
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: b562e925-152b-422a-a90d-626bb2350a65

📥 Commits

Reviewing files that changed from the base of the PR and between 551af95 and da807bc.

📒 Files selected for processing (8)
  • posthog/models/activity_logging/activity_log.py
  • products/workflows/backend/models/hog_flow/hog_flow.py
  • products/workflows/backend/models/hog_flow/hog_flow_template.py
  • products/workflows/backend/models/hog_flow_batch_job/hog_flow_batch_job.py
  • products/workflows/backend/models/hog_flow_revision.py
  • products/workflows/backend/models/hog_flow_schedule/hog_flow_schedule.py
  • products/workflows/backend/models/team_workflows_config.py
  • products/workflows/backend/models/workflow_proposal.py
💤 Files with no reviewable changes (1)
  • posthog/models/activity_logging/activity_log.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Workflow model relationships to teams and users now set related_name="+", disabling their reverse accessors. A new migration records these field changes. The migration pointer and baseline entries are updated. The activity-log field exclusions no longer include resolved_workflow_proposals.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to da807

This change disables unused reverse accessors on workflow relations and adds a migration that produces no SQL. There is no expected user-facing change, and no concrete merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to da807

The change removes ways to reach workflow records from Team and User objects without changing the forward relationships or their deletion policies. No new security exposure was identified, but use of the removed names and deployment behavior are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected boundary is ORM reachability between core Team or User objects and workflow rows. The changed declarations remove reverse paths rather than granting a caller additional access to those rows.

Trust Boundaries and Controls

  • observed — Proposal tenant ownership remains tied to its workflow at save time. The examined relation changes do not alter that assignment or introduce an authentication or authorization entrypoint.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is complete and stand-alone. It covers the problem, changes, testing, known unrelated failure, release status, documentation status, and agent context. Some optional agent gates, such …
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@mayteio mayteio added the reviewhog ($$$) Reviews pull requests before humans do label Sep 29, 2026
@posthog

posthog Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time. Enjoy the moment:

Salad Fingers holds a rusty spoon

Resolved comments: 1 fixed

@posthog posthog Bot removed the reviewhog ($$$) Reviews pull requests before humans do label Sep 29, 2026
Sealing WorkflowProposal.resolved_by with related_name="+" hides the reverse relation, so User._meta.get_fields() no longer lists it and the User activity-log exclusion can never match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: f923bab8-96e1-44de-be46-3be7a5d3e83f
@mayteio mayteio added the stamphog Request AI approval (no full review) label Sep 29, 2026
@mayteio
mayteio marked this pull request as ready for review September 29, 2026 17:27
@mayteio
mayteio requested a review from a team as a code owner September 29, 2026 17:27
@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Sep 29, 2026

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

Not approved yet — waiting on the conditions below.

Re-add the stamphog label to request another review once you have addressed this.

Two gates refused this pull request. The deny-list gate flagged it because it adds a migration (products/workflows/backend/migrations/0027_seal_reverse_accessors.py, along with the max_migration.txt update), and migrations always need a human reviewer. The tier gate classified it as T2-never (180 lines across 11 files, spanning two areas: the workflows models plus posthog/models/activity_logging/activity_log.py and the crossings baseline), a tier that is never auto-approved.

Please ask a human reviewer, ideally someone who owns migrations or the workflows product, to take a look. The size gate passed, so splitting the change is not required for size reasons.

  • 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✗ matches: migrations
size ✓ 166L, 9F substantive, 180L/11F incl. docs/generated/snapshots — within ceiling
tier ✗ classified as T2-never: T2-never (180L, 11F, two-areas, chore)
stamphog 2.3.1 .stamphog/policy.yml @ unknown · reviewed head 67b009a

@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

  • @PostHog/team-platform-features (posthog/models/owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). For a skipped owner, the locator is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

@mayteio
mayteio changed the base branch from master to posthog/workflows-facade-provider-contracts September 29, 2026 17:47
@trunk-io

trunk-io Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

This branch has not been deployed

No deployments
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