UN-3057 [FIX] Repair Prompt Studio projects left ownerless by the clone path - #2239
Open
pk-zipstack wants to merge 1 commit into
Open
UN-3057 [FIX] Repair Prompt Studio projects left ownerless by the clone path#2239pk-zipstack wants to merge 1 commit into
pk-zipstack wants to merge 1 commit into
Conversation
…ne path Since UN-2202, `_is_resource_owner` consults only ResourceMembership OWNER rows for resources that expose `memberships` (CustomTool does) and no longer falls back to `created_by`. The Prompt Studio clone path never created that row, so every project cloned after 0009_absorb_shared_users ran has no owner at all: still visible (the clone copies the parent's `shared_to_org`) and profiles can still be created, but `IsParentToolOwner` denies every mutation on them — deleting an LLM profile returned 403. The clone helper itself is fixed in unstract-cloud; that stops new breakage but cannot help rows already written. This adds the repair: - `repair_ownerless_owner_rows()` grants `created_by` an OWNER row on resources that have zero OWNER rows. Only ownerless resources are touched, so a creator deliberately replaced by a co-owner is not resurrected, and a null creator or null organization is skipped. Idempotent. - It iterates `_base_manager`: several resources' default manager is org-scoped by `UserContext`, which is unset during a migration and would silently filter every row out. Same guard `tenant_account_v2.signals` uses. - Migration 0011 applies it to CustomTool; reverses to a no-op. Regression tests pin all three branches (repair, leave-alone, skip). Verified red-green: with the helper body stubbed to a no-op the behaviour test fails and the two guard tests still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Contributor
|
| Filename | Overview |
|---|---|
| backend/tenant_account_v2/migrations/_membership_backfill.py | Adds the reusable ownerless-resource repair, but fails to promote an existing creator VIEWER membership to OWNER. |
| backend/prompt_studio/prompt_studio_core_v2/migrations/0011_repair_ownerless_custom_tools.py | Correctly wires the repair helper into the Prompt Studio migration graph with a no-op reverse. |
| backend/prompt_studio/prompt_studio_core_v2/tests/test_ownerless_owner_repair.py | Covers ordinary repair and skip cases but omits the existing-creator-VIEWER case that exposes the repair defect. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Migration 0011] --> B[Load CustomTool rows with non-null creator]
B --> C{Organization present?}
C -- No --> D[Skip]
C -- Yes --> E{Any OWNER membership?}
E -- Yes --> F[Leave unchanged]
E -- No --> G[get_or_create creator membership]
G --> H{Membership already exists?}
H -- No --> I[Create OWNER]
H -- Yes, VIEWER --> J[VIEWER remains unchanged]
J --> K[Tool remains ownerless]
Prompt To Fix All With AI
### Issue 1
backend/tenant_account_v2/migrations/_membership_backfill.py:109-114
**Existing viewer remains ownerless**
When an ownerless tool's creator already has a VIEWER membership, `get_or_create` returns that row without applying the OWNER default, causing ownership-gated PUT, PATCH, and DELETE operations to continue returning 403 after the repair.
```suggestion
_, created = Membership.objects.update_or_create(
content_type=content_type,
object_id=object_id,
user_id=resource.created_by_id,
defaults={"role": OWNER, "organization_id": resource.organization_id},
)
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "UN-3057 [FIX] Repair Prompt Studio proje..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+109
to
+114
| _, created = Membership.objects.get_or_create( | ||
| content_type=content_type, | ||
| object_id=object_id, | ||
| user_id=resource.created_by_id, | ||
| defaults={"role": OWNER, "organization_id": resource.organization_id}, | ||
| ) |
Contributor
There was a problem hiding this comment.
When an ownerless tool's creator already has a VIEWER membership, get_or_create returns that row without applying the OWNER default, causing ownership-gated PUT, PATCH, and DELETE operations to continue returning 403 after the repair.
Suggested change
| _, created = Membership.objects.get_or_create( | |
| content_type=content_type, | |
| object_id=object_id, | |
| user_id=resource.created_by_id, | |
| defaults={"role": OWNER, "organization_id": resource.organization_id}, | |
| ) | |
| _, created = Membership.objects.update_or_create( | |
| content_type=content_type, | |
| object_id=object_id, | |
| user_id=resource.created_by_id, | |
| defaults={"role": OWNER, "organization_id": resource.organization_id}, | |
| ) |
Knowledge Base Used: Django Core Scaffold: Settings, Routing, Auth, Tenancy
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/tenant_account_v2/migrations/_membership_backfill.py
Line: 109-114
Comment:
**Existing viewer remains ownerless**
When an ownerless tool's creator already has a VIEWER membership, `get_or_create` returns that row without applying the OWNER default, causing ownership-gated PUT, PATCH, and DELETE operations to continue returning 403 after the repair.
```suggestion
_, created = Membership.objects.update_or_create(
content_type=content_type,
object_id=object_id,
user_id=resource.created_by_id,
defaults={"role": OWNER, "organization_id": resource.organization_id},
)
```
**Knowledge Base Used:** [Django Core Scaffold: Settings, Routing, Auth, Tenancy](https://app.greptile.com/zipstack/-/custom-context/knowledge-base/zipstack/unstract/-/docs/backend-django-core.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Contributor
Unstract test resultsPer-group results
Critical paths
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a data migration repairing Prompt Studio projects that were left with no owner by the clone path, plus the reusable helper behind it.
Paired with Zipstack/unstract-cloud#1721, which fixes the clone helper itself. This PR only repairs rows already written; that one stops new breakage.
Why
Since UN-2202 (
3653b418c),created_byis audit-only:_is_resource_ownertakes the membership branch for any resource exposingmemberships—CustomTooldoes (models.py:195) — and never falls back tocreated_by.The clone helper never created the OWNER
ResourceMembershiprow, so every project cloned after0009_absorb_shared_usersran is ownerless. That backfill seeded OWNER rows fromcreated_byfor everything existing at the time, which is exactly why the regression window starts there.The symptom users hit: the clone is visible (it copies the parent's
shared_to_org) and profiles can still be created (that route uses the looserIsOwnerOrSharedUserOrSharedToOrg), butIsParentToolOwnerdenies DELETE/PUT/PATCH — "unable to delete the LLM profile of a cloned project" returned 403.How
repair_ownerless_owner_rows()grantscreated_byan OWNER row on resources with zero OWNER rows. A creator deliberately replaced by a co-owner is not resurrected; null creator / null organization are skipped. Idempotent._base_manager, notobjects— several resources' default manager is org-scoped byUserContext, which is unset during a migration and would silently filter every row out and repair nothing. Same guardtenant_account_v2.signalsalready documents.0011applies it toCustomTooland reverses to a no-op.Testing
Three regression tests pin repair / leave-alone / skip.
Verified red-green rather than assumed: with the helper body stubbed to
return 0, the behaviour test fails and the two guard tests still pass (they assert the repair must not act).One pre-existing local failure unrelated to this change:
plugins/notification/tests/test_sharing_notification.pyerrors withModuleNotFoundError: No module named 'sendgrid'(chain:test → sharing_notification → email_service → sendgrid; the package is absent from the local venv).Reviewer notes
Two adjacent defects in the same clone path were found but deliberately not fixed here, to keep this scoped to the reported bug — worth separate tickets:
ProfileManager.created_bystill points at the original project's owner, sovalidate_profile_manager_owner_accessevaluates adapter access against the wrong user.shared_to_org=Truefrom the parent, so a clone of a shared project is silently org-wide.🤖 Generated with Claude Code