Skip to content

fix(file-system): drop the retired desktop folder tables - #122447

Merged
trunk-io[bot] merged 1 commit into
masterfrom
eli/drop-retired-desktop-file-system-tables
Oct 23, 2026
Merged

trunk-io[bot] merged 1 commit into
masterfrom
eli/drop-retired-desktop-file-system-tables

Conversation

@eli-r-ph

Copy link
Copy Markdown
Contributor

Problem

  • Deleting a team, user or folder fails if anyone on it ever saved desktop folder instructions.
  • The team deletion workflow retries the failure until it gives up, so the team is never deleted.
  • Cause: 1285_drop_desktop_file_system.py (feat(canvas): build and securely deliver cloud artifacts #73874) removed two models from Django state but kept their tables and foreign keys.
  • Those keys point at posthog_team, posthog_user and posthog_filesystem and are DEFERRABLE INITIALLY DEFERRED. Django no longer cascades into the tables, so the delete fails at COMMIT.

Changes

  • Team, user and folder deletes that failed on these rows now succeed.
  • 1412 drops posthog_filesystemfolderinstructions and posthog_filesystemfoldercontextgeneration with SafeDropTable, the step the table retirement guide says is owed.
  • It depends on canvas.0003_migrate_desktop_tree, which copies these rows into the channel models, so no database drops them before the copy.
  • Databases built from the squashed history never had the tables, and SafeDropTable skips a missing table.

Note

SafeDropTable briefly takes ACCESS EXCLUSIVE on posthog_team, posthog_user and posthog_filesystem, under a budget of half of deadlock_timeout. If it loses the race, the migration gives up and bin/migrate retries it, so live reads are never the deadlock victim (#100779).

How did you test this code?

  • On a fresh database: migrating from zero applies 1412 as a no-op.
  • On a database with both tables recreated from the production DDL: folder, user and team deletes fail with the production error before 1412 and succeed after it. A rerun of 1412 is a no-op.
  • Captured SQL: one LOCK TABLE posthog_filesystem, posthog_team, posthog_user, <both tables> IN ACCESS EXCLUSIVE MODE under a 500ms lock_timeout and statement_timeout, then DROP TABLE IF EXISTS.
  • analyze_migration_risk: "Needs Review", properly staged; makemigrations --check: no changes.

Test rationale: no new test. SafeDropTable has its own tests, and this migration only names tables.

Release status

  • No feature flag controls this change

Docs update

None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

🤖 Generated with Claude Code

1285 removed FileSystemFolderInstructions and FileSystemFolderContextGeneration
from Django state but kept their tables and their deferred foreign keys to
posthog_team, posthog_user and posthog_filesystem. Django no longer
cascades into them, so deleting a team, user or folder they reference fails
at COMMIT. SafeDropTable drops both tables after canvas.0003 has copied
their rows.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@eli-r-ph eli-r-ph self-assigned this Oct 23, 2026
@trunk-io

trunk-io Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Oct 23, 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) — 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.

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • Save view (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

⚠️ Django migration SQL — 1 new migration to review

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

posthog/migrations/1412_drop_retired_desktop_file_system_tables.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-23T15:53:44.681368Z [error    ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=6395 tid=140472029330304
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;
--
-- Drop table posthog_filesystemfoldercontextgeneration, posthog_filesystemfolderinstructions under a deterministic lock order
--
-- (no-op)
COMMIT;

Last updated: 2026-10-23 15:54 UTC (1bdb9e6)

✅ 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

posthog.1412_drop_retired_desktop_file_system_tables
  └─ #1 ⚠️ SafeDropTable
     SafeDropTable - properly staged (prior state removal found)
     tables: ['posthog_filesystemfolderinstructions', 'posthog_filesystemfoldercontextgeneration']

📚 How to Deploy These Changes Safely

SafeDropTable:

✅ Validated staged drop: Found prior SeparateDatabaseAndState that removed each model from state.

Remaining checklist is the one for any staged drop: all code references removed, one full deployment cycle waited since the state removal, and no other table referencing these.

See the migration safety guide

Last updated: 2026-10-23 15:55 UTC (1bdb9e6)

@trunk-io

trunk-io Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

Failed Test Failure Summary Logs
SQL Editor › Basic flow › Save view The test failed because the expected element was not visible. Logs ↗︎

View Full Report ↗︎ ⋅ Docs

@eli-r-ph
eli-r-ph marked this pull request as ready for review October 23, 2026 15:50
@parameterai

parameterai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Risk: No findings

This PR adds one migration (posthog.1412) that drops the two retired desktop file-system tables left behind by migration 1285, using the existing SafeDropTable helper and depending on canvas.0003 so the row copy into the channel models runs first. I traced the ordering, the lock phase, and the dependency resolution and found no security issue: no attacker-controlled input, no auth surface, and the drop fails closed if the copy migration cannot run.

Sentinel reviewed 1bdb9e6 · Review settings

@trunk-io
trunk-io Bot merged commit 44e074e into master Oct 23, 2026
376 checks passed
@trunk-io
trunk-io Bot deleted the eli/drop-retired-desktop-file-system-tables branch October 23, 2026 16:35
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-23 17:08 UTC Run
prod-us ✅ Deployed 2026-10-23 18:09 UTC Run
prod-eu ✅ Deployed 2026-10-23 18:17 UTC Run

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