feat(data-warehouse): make the project warehouse connection read/write

[fuziontech]

What

A project's managed-warehouse connection in the SQL editor now uses Duckgres's project_user (posthog_team_<id>_rw) instead of the read-only project_reader (posthog_team_<id>). Writes are the default because a project connection is the user's own warehouse, not a report of it.

⚠️ Merge order: duckgres PR #999 must be deployed FIRST

PostHog/duckgres#999 adds the /teams/:id/project-user endpoint this calls. It is not deployed yet.

If this merges ahead of it, the failure is not benign: ensure_managed_warehouse_direct_source sees the credential_kind mismatch, re-credentials the source and drops its discovered catalog, sets direct_query_enabled=False, and only then calls the handshake — which 404s. Existing project connections would go dark (no tables, not queryable) and stay that way until duckgres ships. Please hold this PR until #999 is out.

The project boundary does not change

Duckgres derives the same namespaces for both modes — the org-team row's schema_name, its data-imports schema, shadow_<team>_models, and the legacy posthog.<events|persons> overrides. Write authorization does not widen the set of reachable relations, so a project still cannot read or write another project's schemas.

That is why project_reader_namespaces became project_namespaces rather than gaining a mode argument — the mirror is correct for both modes, and it's documented as mode-independent so nobody "fixes" it later by forking it per mode.

Migration of existing sources

Reuses the mechanism already built for the earlier root → project_reader move. connection_metadata.credential_kind is the marker, now a named constant (PROJECT_CREDENTIAL_KIND) with a comment saying that changing its value re-runs the handshake fleet-wide:

• kind mismatch → re-credential, drop the catalog discovered as the old principal, direct_query_enabled=False
• handshake against duckgres confirms the new credential → re-enable, reconcile rediscovers tables as the new principal

So a half-migrated source is never queryable. The old read-only posthog_team_<id> login stays behind in duckgres — harmless (strictly less privileged, and PostHog no longer stores it), but it is credential sprawl worth a cleanup pass later.

Tests

Added both directions of the new migration, alongside the existing root-upgrade test:

• test_upgrades_a_read_only_project_reader_source_to_the_read_write_login — re-credentials to _rw, drops the stale catalog
• test_leaves_a_source_already_on_the_read_write_login_untouched — the handshake is one-time, not re-minted on every sweep (which would churn the password and break in-flight queries)

Test plan

• ruff check + ruff format clean
• mypy clean on both changed modules (the repo-wide errors it reports are pre-existing, in unrelated files)
• The 6 adapter tests in tests/api/test_managed_warehouse.py pass locally
• DB-backed tests in tests/test_managed_warehouse_connection.py not run locally — the session fixture needs sqlx-cli for the separate persons database, which isn't installed in my environment. Relying on CI for these, including the two added above.

🤖 Generated with Claude Code

https://claude.ai/code/session_018YWtQ3UqgzFsrqJSquTZoM

[github-actions]

Hey @fuziontech! 👋

It looks like your git author email on this PR isn't your @posthog.com address (fuziontech@gmail.com). Since you're on the PostHog team, it's worth pointing your local git author email at your @posthog.com address. Why it matters:

• Consistent work identity in git history — internal tooling that attributes commits to team members keys off your @posthog.com address.
• Keeps team contributions easy to tell apart from external community ones when scanning history.

You can fix it for this repo with:

git config user.email "you@posthog.com"

Or set it globally with git config --global user.email "you@posthog.com". No need to redo this PR — just a nudge for next time. 🙂

[greptile-apps]

Failed handshake disables existing sources

When an existing source is reconciled before /teams/:id/project-user is available, the credential-kind migration commits catalog deletion and direct_query_enabled=False before this request fails, causing the connection to lose its tables and remain unavailable until a later successful sweep.

Knowledge Base Used: Batch exports and data warehouse bulk data movement

Prompt To Fix With AI

This is a comment left during a code review.
Path: products/data_warehouse/backend/presentation/views/managed_warehouse.py
Line: 253

Comment:
**Failed handshake disables existing sources**

When an existing source is reconciled before `/teams/:id/project-user` is available, the credential-kind migration commits catalog deletion and `direct_query_enabled=False` before this request fails, causing the connection to lose its tables and remain unavailable until a later successful sweep.

**Knowledge Base Used:** [Batch exports and data warehouse bulk data movement](https://app.greptile.com/posthog-org-19734/-/custom-context/knowledge-base/posthog/posthog/-/docs/batch-exports-warehouse.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[fuziontech]

Agreed, and thanks — this is the same hazard Copilot raised, and it is the one I called out in the PR description as a hard deploy-order requirement: PostHog/duckgres#999 must be deployed before this merges.

Full analysis in my reply to Copilot, short version: the obvious fix (handshake before mutating) would regress the concurrency property test_concurrent_credential_setup_reuses_the_persisted_credential pins — the DB write is the claim that stops a concurrent sweep minting a second password, so reordering it can leave the persisted password and the one Duckgres stored permanently out of sync. A correct fix needs the new credential staged and promoted after the handshake, which is design work rather than a review-comment change.

Current plan is to gate on deploy order (the feature cannot function before #999 ships anyway, and the state self-heals on the next sweep once Duckgres is up). If the team prefers not to rely on sequencing, I will put the migration behind a flag and flip it post-deploy.

[greptile-apps]

The concurrency argument holds — the DB write being the claim is load-bearing for test_concurrent_credential_setup_reuses_the_persisted_credential, so naively reordering it trades one hazard for another. Your analysis is correct.

Between the two options: the deploy-order gate is fine given that the PR description already calls it out prominently with a hard warning, and the self-healing property means the window of breakage is bounded to the time between this merge and duckgres #999 landing. That's an acceptable operational risk as long as the merge isn't automated.

The flag approach is cleaner if there's any chance of the sequencing slipping (e.g., the duckgres PR stalls, someone merges this during an incident window without checking), but it does add a cleanup step. Given you've already documented the hazard clearly in the PR description, the deploy-order gate seems sufficient here — no further change needed from a review standpoint.

[greptile-apps]

Prompt To Fix All With AI

### Issue 1
products/data_warehouse/backend/presentation/views/managed_warehouse.py:253
**Failed handshake disables existing sources**

When an existing source is reconciled before `/teams/:id/project-user` is available, the credential-kind migration commits catalog deletion and `direct_query_enabled=False` before this request fails, causing the connection to lose its tables and remain unavailable until a later successful sweep.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "feat(data-warehouse): make the project w..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR updates the managed-warehouse SQL editor connection so each project uses Duckgres’s read/write project_user credential (instead of the read-only project_reader), and updates the provisioning mirror/helpers and tests to match.

Changes:

• Switch credential provisioning from /teams/:id/project-reader to /teams/:id/project-user and rename helpers accordingly.
• Introduce PROJECT_CREDENTIAL_KIND = "project_user" as the migration marker for re-credentialing existing sources.
• Update and extend test coverage to validate upgrading legacy project_reader sources to the new read/write login and keeping already-upgraded sources stable.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
products/data_warehouse/backend/managed_warehouse_connection.py	Switches the managed connection to project_user, adds the credential-kind marker, and updates reconciliation to use the renamed namespace mirror.
products/data_warehouse/backend/presentation/views/managed_warehouse.py	Renames and updates the Duckgres adapter functions to provision project_user and exposes mode-independent project_namespaces.
products/data_warehouse/backend/tests/api/test_managed_warehouse.py	Updates adapter tests for the new endpoint/name and verifies namespace mirroring remains correct.
products/data_warehouse/backend/tests/test_managed_warehouse_connection.py	Updates managed-connection tests and adds migration tests for upgrading project_reader sources to project_user.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[fuziontech]

Correct diagnosis, and greptile flagged the same thing — this is the hazard I called out in the PR description as a hard deploy-order requirement. Agreed it is real: on a kind mismatch the source is re-credentialed, disabled and its catalog dropped before the handshake, so a 404 leaves existing connections dark.

On the two suggested fixes, though — I think the first one would regress a property this file is deliberately built around, and I want to flag that before anyone implements it.

"Mint credentials first, then atomically swap" breaks the concurrency design. The DB write is not just bookkeeping, it is the claim: ensure_managed_warehouse_direct_source persists the credential under select_for_update so a concurrent sweep reads it back and reuses it instead of minting a second password. test_concurrent_credential_setup_reuses_the_persisted_credential pins exactly that (both re-entrant calls must request the same password). If the handshake moves first, two sweeps mint different passwords; Duckgres's upsert is last-write-wins and so is the row write, and the two can interleave such that the persisted password is not the one Duckgres ended up storing — a permanently broken connection, which is worse than the transient one we are trying to avoid.

"Treat 404 as keep using project_reader" does not help by itself, because by the time the 404 comes back the wipe has already committed. It would have to be combined with deferring the mutation, which lands back on the problem above.

A fix that preserves the claim-first property would need the new credential staged separately (e.g. a pending-credential field) and promoted only after a successful handshake — real design work, not a review-comment change.

Given that, my inclination is to keep the current behaviour and treat deploy order as the control, because:

• the PR cannot deliver anything until feat(controlplane): add project_user, a read/write project-scoped login duckgres#999 ships regardless — the endpoint has to exist for the feature to work at all;
• the exposure is bounded to the rollout window, and self-heals on the next sweep after Duckgres is up;
• the wipe-then-handshake shape is pre-existing (it is how the earlier root -> project_reader migration worked), so this is not a new class of risk, just a newly-triggered one.

If the team would rather not depend on deploy sequencing, the cheapest safe alternative is to merge this behind a flag and flip it after Duckgres is deployed. Happy to do that — flagging the tradeoff rather than picking unilaterally.

[fuziontech]

Self-review notes for reviewers

Three things worth knowing before you read the diff.

1. This grants the capability; it does not yet open a write surface

The credential becomes read/write at the database boundary, but the SQL editor still cannot emit writes — the direct-query path goes through HogQL, whose AST has 106 node classes and no write statements (no Insert/Update/Delete/Create/…), and execute_hogql_query only accepts SelectQuery | SelectSetQuery.

So merging this changes nothing a user can observe today. It's the foundation; surfacing writes is a separate piece of work in the query layer. Flagging it so nobody merges this expecting CREATE TABLE to start working in the editor.

2. Duckgres-side verification is done and green

The dependency PR (PostHog/duckgres#999) passed its full e2e against the real mw-dev cluster, including the new project_user_isolation assertion — in-project DML+DDL round trip, cross-project read and write denial, unqualified-write-target denial, namespace-DDL denial, and a check that the team's reader stays read-only. That was the one gap I couldn't cover locally.

Worth knowing what that assertion exists to catch: reviewing duckgres#999 turned up a real project-boundary escape in its first draft. A write target does not bind to a same-named CTE (only reads do), so WITH shared AS (…) INSERT INTO shared VALUES (1) resolved through the session search_path into ducklake.main — a namespace shared across the whole org. Fixed there, with regression tests at both layers.

3. Stale-comment fix pushed

d5def26 corrects _ensure_direct_source's docstring, which still claimed the credential "enforces read-only SQL".

CI

semgrep is red, and it is not this PR: 0 findings / 0 blocking, exit 3 from a PartialParsing error in .github/scripts/verify-playwright-new-tests-and-snapshots.sh (a CI shell script this PR doesn't touch). It's failing identically on #74250, #74240 and #74230.

The DB-backed tests in tests/test_managed_warehouse_connection.py — including the two added here for the reader→user migration — could not run in my environment (the session fixture needs sqlx-cli for the separate persons database), so the Django suites in CI are the first real run of those.

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 1 · PR risk: 0/10

[github-actions]

🤖 CI report

⚠️ Backend coverage — 91.0% of changed backend lines covered — 7 uncovered

🧪 Backend test coverage

Patch coverage — changed backend lines (products + core): ██████████████████░░ 91.0% (76 / 83)

File	Patch	Uncovered changed lines
products/data_warehouse/backend/presentation/views/managed_warehouse.py	50.0%	258, 262
products/data_warehouse/backend/managed_warehouse_connection.py	73.7%	139, 255, 262, 264, 283

🤖 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 30437066389 -n patch-coverage), or the coverage-data block at the end of this comment.

Per-product line coverage (touched products)

Product	Coverage	Lines
demo	███████████░░░░░░░░░ 56.2%	1,497 / 2,663
tasks	██████████████░░░░░░ 69.7%	31,432 / 45,123
cdp	████████████████░░░░ 81.0%	3,146 / 3,883
signals	████████████████░░░░ 81.1%	23,838 / 29,387
data_modeling	█████████████████░░░ 85.3%	7,343 / 8,611
notebooks	█████████████████░░░ 85.7%	7,525 / 8,785
actions	█████████████████░░░ 86.6%	717 / 828
cohorts	█████████████████░░░ 86.9%	5,648 / 6,496
product_tours	██████████████████░░ 87.9%	1,303 / 1,482
data_warehouse	██████████████████░░ 88.3%	12,169 / 13,787
exports	██████████████████░░ 88.4%	6,950 / 7,862
dashboards	██████████████████░░ 89.4%	5,983 / 6,693
engineering_analytics	██████████████████░░ 89.4%	6,441 / 7,202
conversations	██████████████████░░ 89.6%	17,234 / 19,241
alerts	██████████████████░░ 90.0%	4,342 / 4,827
mcp_analytics	██████████████████░░ 90.2%	2,883 / 3,198
streamlit_apps	██████████████████░░ 90.7%	2,630 / 2,901
error_tracking	██████████████████░░ 91.0%	10,925 / 12,004
slack_app	██████████████████░░ 91.1%	9,554 / 10,492
stamphog	██████████████████░░ 91.1%	4,056 / 4,450
marketing_analytics	██████████████████░░ 91.2%	12,058 / 13,222
product_analytics	███████████████████░ 92.5%	5,849 / 6,321
early_access_features	███████████████████░ 92.6%	1,287 / 1,390
ai_observability	███████████████████░ 92.8%	15,328 / 16,517
surveys	███████████████████░ 93.1%	5,771 / 6,197
web_analytics	███████████████████░ 93.2%	14,826 / 15,906
posthog_ai	███████████████████░ 93.2%	1,326 / 1,422
approvals	███████████████████░ 93.3%	3,437 / 3,682
reminders	███████████████████░ 93.4%	468 / 501
workflows	███████████████████░ 93.9%	6,919 / 7,372
endpoints	███████████████████░ 94.2%	8,655 / 9,192
skills	███████████████████░ 94.6%	3,158 / 3,337
review_hog	███████████████████░ 94.6%	6,912 / 7,303
experiments	███████████████████░ 95.4%	25,415 / 26,637
logs	███████████████████░ 95.5%	10,435 / 10,928
annotations	███████████████████░ 96.2%	732 / 761
revenue_analytics	███████████████████░ 96.3%	1,887 / 1,960
feature_flags	███████████████████░ 96.4%	17,371 / 18,023
replay_vision	███████████████████░ 96.4%	15,861 / 16,455
user_interviews	███████████████████░ 96.5%	2,638 / 2,734
customer_analytics	███████████████████░ 97.1%	9,798 / 10,086
warehouse_sources	███████████████████░ 97.2%	346,692 / 356,604
data_catalog	████████████████████ 97.7%	2,556 / 2,615
pulse	████████████████████ 98.4%	2,017 / 2,049

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.

[trunk-io]

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