feat: add ownership transfer for packages and extend skill transfers with org support#1388
feat: add ownership transfer for packages and extend skill transfers with org support#1388TommYDeeee wants to merge 25 commits intoopenclaw:mainfrom
Conversation
…ansfers for org support - Make toUserId optional in skillOwnershipTransfers (for org-targeted transfers) - Add fromPublisherId and toPublisherId optional fields to skillOwnershipTransfers - Add new packageOwnershipTransfers table with matching shape - Register packageOwnershipTransfers in the schema export
Add convex/lib/transfers.ts with reusable helpers for both skill and package transfer systems: expiry check, handle normalization, and ownership/accept permission validation with org support.
Use shared helpers from convex/lib/transfers.ts (validateTransferOwnership, validateTransferAcceptPermission, isTransferExpired, normalizeTransferHandle) instead of inline functions. Add toPublisherId/publisherId args for org-targeted transfers, and type: "skill" to listing query results.
Implements convex/packageTransfers.ts mirroring the skill transfer system, with org-targeted transfer support via fromPublisherId/toPublisherId fields and shared validation helpers from convex/lib/transfers.ts.
Add auto-detection logic that probes both skill and package APIs when --type is not specified. Update transfer request, accept, reject, cancel, and list commands with --type and --publisher options. Update response schemas to handle package transfers alongside skill transfers.
Add POST routes for package ownership transfers (request, accept, reject, cancel) mirroring the existing skill transfer pattern. Update the transfers listing endpoint to query both skill and package transfers in parallel.
…errors - Add validateTransferAcceptPermission to package accept/reject handlers - Fix recipient check in validatePendingTransferForActor for org transfers - Make toUserId optional in validateTransferAcceptPermission signature - Remove unused imports in test file
The transfer listing now queries both skill and package transfers, so the mock needs to return data for the first call (skills) and an empty array for the second (packages).
- Forward toPublisherId in HTTP handlers by resolving publisher handle to ID via getByHandleInternal before calling transfer mutations - Forward publisherId on transfer accept in both package and skill HTTP handlers - Fix validatePendingTransferForActor in skillTransfers to allow org-targeted transfers where toUserId is undefined - Remove unsafe `as Id<"users">` cast on transfer.toUserId in acceptTransferInternal and rejectTransferInternal - Fix listOutgoingInternal for skills to include org-targeted transfers instead of skipping them with `if (!transfer.toUserId) continue` - Sort unified transfer listing by requestedAt descending
CLI was sending toPublisherId (handle string) but HTTP handlers read toPublisherHandle and publisherHandle. Fixed CLI to match.
Org-targeted skill transfers were setting toUserId to undefined, making them invisible in incoming transfer lists. Now always set toUserId to the resolved recipient, matching package transfer behavior.
|
@TommYDeeee is attempting to deploy a commit to the 0xBuns Team on Vercel. A member of the Team first needs to authorize it. |
- http-api.md: add package transfer endpoints, org-targeted transfer params (toPublisherHandle, publisherHandle), updated listing response - cli.md: update transfer commands with --type, --publisher flags - api.md: add package transfer endpoint listing - orgs.md: mark transfer model as implemented - CHANGELOG.md: add unreleased entry for transfer features
…ort" This reverts commit fa637da.
…port - http-api.md: add package transfer endpoints, org-targeted transfer params (toPublisherHandle, publisherHandle), updated listing response - cli.md: update transfer commands with --type, --publisher flags - api.md: add package transfer endpoint listing - orgs.md: mark transfer model as implemented
Greptile SummaryThis PR adds a complete package ownership transfer system (mirroring the existing skill transfer pattern) and extends skill transfers with org support — allowing users, org admins, and orgs to transfer packages and skills between each other. The architecture is sound: shared helpers in Critical issue found:
Everything else looks good:
Confidence Score: 4/5Not safe to merge as-is: the stale-ownership check will silently cancel all org-initiated transfers at accept time, making the org transfer feature completely non-functional. One P1 defect affects both new files (packageTransfers.ts and skillTransfers.ts): the stale-ownership comparison uses fromUserId (the requesting actor/admin) against ownerUserId (the package/skill's linked user), which differ for every org-owned item. This breaks the primary new feature — org transfers — end-to-end. All other logic (routing, schema, helpers, CLI, listing) looks correct and well-tested. A targeted two-line fix resolves the issue. convex/packageTransfers.ts (acceptTransferInternal stale check) and convex/skillTransfers.ts (same stale check, carried over from pre-org code)
|
| Filename | Overview |
|---|---|
| convex/packageTransfers.ts | New file implementing the full package transfer lifecycle. Well-structured and mirrors the skill transfer pattern, but acceptTransferInternal has a critical stale-ownership check bug that will break all org-initiated transfers. |
| convex/skillTransfers.ts | Refactored to use shared transfer helpers and extended with org support. The stale-ownership check in acceptTransferInternal carries over the same bug — org-admin-initiated transfers will always be incorrectly cancelled at accept time. |
| convex/lib/transfers.ts | New shared helper library extracting expiry, handle normalisation, and permission validation logic. Clean and well-tested. |
| convex/schema.ts | Adds packageOwnershipTransfers table mirroring skillOwnershipTransfers, and makes toUserId optional on both to support org targets. Indexes look correct. |
| convex/httpApiV1/packagesV1.ts | New POST router for package transfer endpoints. Segment routing logic is correct. Error mapping and auth flows are consistent with the existing skill transfer handlers. |
| convex/httpApiV1/transfersV1.ts | Unified listing merges skill and package transfers with Promise.all and sorts by requestedAt. Clean and correct. |
| packages/clawdhub/src/cli/commands/transfer.ts | CLI extended with --type and --publisher flags and auto-detection via parallel API probes. Logic is sound; the updated transfer list display adds a TYPE column correctly. |
Comments Outside Diff (1)
-
convex/packageTransfers.ts, line 1418-1424 (link)Stale-ownership check breaks org-initiated transfers
transfer.fromUserIdis set toargs.actorUserIdinrequestTransferInternal— i.e. the admin who initiated the transfer. For org-owned packages,pkg.ownerUserIdis the linked user of the org (not the admin), sopkg.ownerUserId !== transfer.fromUserIdis almost alwaystruefor org-initiated transfers, even when ownership hasn't changed. Every org-initiated package transfer will be incorrectly cancelled with "Transfer is no longer valid" at accept time.The check needs to discriminate between personal and org ownership. When
transfer.fromPublisherIdis set the transfer was from an org — compare publisher IDs instead:const ownerChanged = transfer.fromPublisherId ? pkg.ownerPublisherId !== transfer.fromPublisherId : pkg.ownerUserId !== transfer.fromUserId; if (ownerChanged) { await ctx.db.patch(transfer._id, { status: "cancelled", respondedAt: now }); throw new Error("Transfer is no longer valid"); }
The same bug exists in
convex/skillTransfers.tsin theacceptTransferInternalhandler — that stale check was also written before org support existed and has the same issue.Prompt To Fix With AI
This is a comment left during a code review. Path: convex/packageTransfers.ts Line: 1418-1424 Comment: **Stale-ownership check breaks org-initiated transfers** `transfer.fromUserId` is set to `args.actorUserId` in `requestTransferInternal` — i.e. the *admin* who initiated the transfer. For org-owned packages, `pkg.ownerUserId` is the linked user of the org (not the admin), so `pkg.ownerUserId !== transfer.fromUserId` is almost always `true` for org-initiated transfers, even when ownership hasn't changed. Every org-initiated package transfer will be incorrectly cancelled with "Transfer is no longer valid" at accept time. The check needs to discriminate between personal and org ownership. When `transfer.fromPublisherId` is set the transfer was from an org — compare publisher IDs instead: ```typescript const ownerChanged = transfer.fromPublisherId ? pkg.ownerPublisherId !== transfer.fromPublisherId : pkg.ownerUserId !== transfer.fromUserId; if (ownerChanged) { await ctx.db.patch(transfer._id, { status: "cancelled", respondedAt: now }); throw new Error("Transfer is no longer valid"); } ``` The same bug exists in `convex/skillTransfers.ts` in the `acceptTransferInternal` handler — that stale check was also written before org support existed and has the same issue. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: convex/packageTransfers.ts
Line: 1418-1424
Comment:
**Stale-ownership check breaks org-initiated transfers**
`transfer.fromUserId` is set to `args.actorUserId` in `requestTransferInternal` — i.e. the *admin* who initiated the transfer. For org-owned packages, `pkg.ownerUserId` is the linked user of the org (not the admin), so `pkg.ownerUserId !== transfer.fromUserId` is almost always `true` for org-initiated transfers, even when ownership hasn't changed. Every org-initiated package transfer will be incorrectly cancelled with "Transfer is no longer valid" at accept time.
The check needs to discriminate between personal and org ownership. When `transfer.fromPublisherId` is set the transfer was from an org — compare publisher IDs instead:
```typescript
const ownerChanged = transfer.fromPublisherId
? pkg.ownerPublisherId !== transfer.fromPublisherId
: pkg.ownerUserId !== transfer.fromUserId;
if (ownerChanged) {
await ctx.db.patch(transfer._id, { status: "cancelled", respondedAt: now });
throw new Error("Transfer is no longer valid");
}
```
The same bug exists in `convex/skillTransfers.ts` in the `acceptTransferInternal` handler — that stale check was also written before org support existed and has the same issue.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: convex/skillTransfers.ts
Line: 188-191
Comment:
**Same stale-ownership bug as in `packageTransfers.ts`**
`transfer.fromUserId` stores the requesting actor (which is the org admin for org-owned skills), not `skill.ownerUserId`. The comparison `skill.ownerUserId !== transfer.fromUserId` is therefore always `true` when an org admin initiates the transfer, causing every org-initiated skill transfer to be cancelled as stale at accept time.
Apply the same fix described in `convex/packageTransfers.ts`:
```typescript
const ownerChanged = transfer.fromPublisherId
? skill.ownerPublisherId !== transfer.fromPublisherId
: skill.ownerUserId !== transfer.fromUserId;
if (ownerChanged) {
await ctx.db.patch(transfer._id, { status: "cancelled", respondedAt: now });
throw new Error("Transfer is no longer valid");
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: always set toUserId in skill transf..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a14c3cfd6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Remove strikethrough and implementation notes, write as production documentation describing current transfer capabilities.
…alidation - Use fromPublisherId for ownership-changed detection on org transfers. Previously compared ownerUserId against fromUserId (the requesting actor), which always mismatched when an org admin initiated the transfer, incorrectly cancelling valid org transfers at accept time. - Validate actor has admin/owner role on explicit publisherId override in accept flow. Previously any user could assign ownership to an arbitrary publisher by passing publisherId on accept.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
HTTP handlers previously looked up pending transfers by toUserId only, blocking org admins (other than the original toUserId) from accepting or rejecting. Now falls back to an unfiltered pending transfer lookup for the item, letting the mutation's validateTransferAcceptPermission handle authorization.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d19162956
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-detection error - Skip "Cannot transfer to yourself" check when toPublisherId is set, enabling single-admin org owners to transfer items to their own org. - Improve auto-detection error message to suggest --type flag for private items where detail endpoints return 404.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed2f665560
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The recipient gate in validatePendingTransferForActor blocked org admins whose userId didn't match toUserId, even when toPublisherId indicated an org-targeted transfer. Now skips the toUserId check when toPublisherId is set, deferring authorization to validateTransferAcceptPermission which checks org membership.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4022b0bc7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ccept body - validateTransferAcceptPermission now checks isPublisherActive before allowing org-targeted acceptance, preventing ownership assignment to deactivated/deleted publishers. - Accept HTTP handlers now return a parse error when a JSON body is sent but malformed, instead of silently falling through to personal publisher.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d5939fd96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Cancel was gated on fromUserId === actorUserId, blocking other org admins from cancelling transfers initiated by a different admin. Now skips the direct sender check when fromPublisherId is set and validates org membership via validateTransferOwnership instead.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27dfd873bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Cancel HTTP handlers only looked up transfers by fromUserId, blocking other org admins. Now falls back to unfiltered pending transfer lookup, matching the accept/reject pattern. Authorization is enforced by validateTransferOwnership in the mutation layer.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19e7951eb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The packages detail endpoint falls back to returning skills when no package exists, causing auto-detection to incorrectly report both a skill and package for skill-only items. Now parses the response body to confirm a package field is present before treating it as a package match.
|
This would solve #1431 |
Resolve conflicts in http.ts and httpApiV1.ts. Merge the new trusted-publisher and packages DELETE routes from main with our transfer POST routes into a unified packagesPostRouterV1Handler. Remove duplicate runMutationRef and getPackageByNameInternal entries.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 517778d634
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
apiRequest already calls JSON.stringify on the body. Passing a pre-stringified body caused double-encoding, so the server never saw the publisherHandle/toPublisherHandle fields and silently fell back to the default publisher.
2 participants
Summary
Adds the ability to transfer ownership of packages between users and organizations, and extends the existing skill transfer system with the same org support. This enables publishers to move packages/skills they own to another user or to an organization they admin — a flow that was previously impossible after initial publishing.
Motivation
Once a package was published under a personal account, there was no way to reassign it to an organization. Users who published packages under their personal handle and later wanted to move them under their org were stuck. This PR solves that by introducing a transfer system for packages (mirroring the existing skill transfer pattern) and extending skill transfers to support org targets.
What's new
Package transfers (new)
Org-targeted transfers (new for both skills and packages)
adminorownerrole members can request/accept org transfersExtended skill transfers
CLI updates
--type skill|packageflag — explicit type selection, auto-detects when omitted--publisher @org-handleflag — target an org on request, or assign to org on accept--typetransfer listnow shows both skill and package transfers with a TYPE column, sorted by dateAPI endpoints
/api/v1/packages/{name}/transfertoUserHandle, optionaltoPublisherHandle,message)/api/v1/packages/{name}/transfer/acceptpublisherHandleto assign to org)/api/v1/packages/{name}/transfer/reject/api/v1/packages/{name}/transfer/cancel/api/v1/transfers/incoming/api/v1/transfers/outgoingExisting skill transfer endpoints unchanged —
toPublisherHandleandpublisherHandleare optional additions.Documentation updates
docs/http-api.md— added package transfer endpoints, org-targeted transfer params (toPublisherHandle,publisherHandle), updated transfer listing response shape withtypediscriminatordocs/cli.md— updated transfer command reference with--type,--publisherflags and package supportdocs/api.md— added package transfer endpoints to the API endpoint listingdocs/orgs.md— updated transfer model section to reflect implemented publisher-based flowsArchitecture
packageOwnershipTransfersmirrorsskillOwnershipTransfers— same shape, same indexes, separate tables. Lower risk than a unified table, no refactoring of working skill code.convex/lib/transfers.tsextracts common logic (expiry, handle normalization, ownership validation, accept permission checks) used by both systems.fromPublisherId/toPublisherIdoptional fields toskillOwnershipTransfers. NewpackageOwnershipTransferstable with same structure.Files changed
convex/lib/transfers.tsconvex/lib/transfers.test.tsconvex/packageTransfers.tsconvex/packageTransfers.test.tsconvex/schema.tspackageOwnershipTransferstable, extendedskillOwnershipTransfersconvex/skillTransfers.tsconvex/skillTransfers.test.tsconvex/httpApiV1/packagesV1.tsconvex/httpApiV1/skillsV1.tsconvex/httpApiV1/transfersV1.tsconvex/httpApiV1.tsconvex/http.ts/api/v1/packages/prefixpackages/clawdhub/src/cli/commands/transfer.tspackages/clawdhub/src/cli.tspackages/clawdhub/src/schema/schemas.tsconvex/httpApiV1.handlers.test.tsdocs/http-api.mddocs/cli.mddocs/api.mddocs/orgs.mdLinked issue
This would close #1431
Test plan
--publisherflag to assign to org--typefallback