Milab 2668/project sharing#1719
Conversation
- Introduced new types and models for sharing, including `OutgoingShare`, `PendingShare`, and `EnvelopeData`. - Implemented functions for creating and managing shares, including `buildShareEnvelope`, `writeEnvelopeAcceptance`, and `writeSharingDecision`. - Added support for managing shared resources with `SynchronizedTreeState` for both donors and acceptors. - Enhanced the middle layer's ops settings to include `envelopeTtlMs` for envelope expiration management. - Created computables for reactive views of outgoing and pending shares, allowing for dynamic updates based on user interactions. - Updated the tree state management to support multi-root trees, enabling better handling of shared resources. - Added new utility functions for handling resource IDs and project adoption from shared envelopes.
- Added `getSessionInfo` method to retrieve the authenticated caller's session ID and role, supporting both gRPC and REST. - Introduced `listGrants` method to enumerate grants for a specific resource, ensuring only resource owners can access it via gRPC. - Updated `UserResources` to include `listGrants` and `listUsers` methods, with the latter currently returning a stubbed response. - Enhanced `MiddleLayer` to expose user roles and sharing capabilities, including the ability to share resources with everyone. - Refactored sharing logic to support both targeted and public sharing, with appropriate handling of grants and envelope management. - Introduced `ShareId` type for better identification of shares across the system. - Updated sharing model to include role-based permissions for public grants. - Improved performance of tree polling for user resources to optimize discovery of shared resources.
…nage recipient grants
🦋 Changeset detectedLatest commit: f01c92d The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces project-sharing capabilities (Copy & Share) across the platform, updating the API definitions, pl-client, pl-middle-layer, and pl-tree to support multi-root synchronized trees and dynamic root discovery. The review feedback focuses on performance and robustness improvements: avoiding a redundant gRPC call at startup by initializing the loop iteration counter to 1, parallelizing sequential resource reads using Promise.all during envelope cleanup and supersede searches, and enhancing the outgoing shares computable to gracefully handle REST clients and filter out stale recipient responses.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ents and server-wide sharing
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
| try { | ||
| this._currentUserRole = (await this._ll.getSessionInfo()).role; | ||
| } catch { | ||
| this._currentUserRole = null; |
There was a problem hiding this comment.
Let's not rely on exception here, and clearly write the conditions when it should be null, I guess it is not only anonymous, but also for older backend versions?
| const cl = this.clientProvider.get(); | ||
|
|
||
| if (!(cl instanceof GrpcPlApiClient)) { | ||
| throw new Error("ListUsers requires gRPC wire protocol; REST is not supported"); |
There was a problem hiding this comment.
Are you sure this is a runtime check? In other words if you instantiated new client (packed with this version), but back-end does not support it, then cl instanceof GrpcPlApiClient will be false? Please confirm, looks suspicious, most probably you need to use feature flags from the backend here.
There was a problem hiding this comment.
The only caller gates on the capability
| * ({@link GrantType.MAKE_RESOURCE_PUBLIC}) grant is recorded against this login, | ||
| * and recipients of an everyone-grant surface in `ListGrants` with this value — | ||
| * callers map it to "*". Mirror of `EveryoneUser` in | ||
| * `pl/platform/model/user.go`. |
There was a problem hiding this comment.
Please don't cite back-end code in here.
| * callers map it to "*". Mirror of `EveryoneUser` in | ||
| * `pl/platform/model/user.go`. | ||
| */ | ||
| export const EveryoneUser = |
There was a problem hiding this comment.
Confirmed with @DenKoren, not a part of public contract, we don't need it here.
There was a problem hiding this comment.
unfortunately right know it's only one way
There was a problem hiding this comment.
incorrect, see comment from Denis below
|
|
||
| async function loadTreeStateViaBfs( | ||
| tx: PlTransaction, | ||
| loadingRequest: TreeLoadingRequest, |
There was a problem hiding this comment.
To merge the grand listing logic the idea is basically to move it here, so, just adding a list of those grants in the request and they will be polled within the same transaction using transactional version of list grants.
There was a problem hiding this comment.
addressed in a different way
…nce functionality
| export function isEveryoneUserLogin(login: string): boolean { | ||
| return login.length === EveryoneLoginLength && login.startsWith(EveryonePrefix); | ||
| } |
There was a problem hiding this comment.
Unfortunately, we have to hardcode the detection logic, you're right :(. Better to stick to a constant you tried earlier :( (everyone-Po9ahwahxai7Aejingaiyiequuecu3ei4moaNge4xahTh0Co7XeeLeiph6ahy3As). It will never change in backend and I will bring the right way of detection via grant type. We'll just wire new capability detection and 'everyone-*' user will never be exposed to API in modern backends.
We already have a version of backend that released with a bug exposing 'everyone' to the API, so we can't avoid checking this completely, but we know this constant will not change in API.
It's OK to write this const on a fence. There are guards for this const in code that prevent its usage of it in API in critical places.
MILAB-2668: refresh API, bring constants renaming
…t reads Revoking an everyone-share tore down the client session: a revoked resource signature comes back as UNAUTHENTICATED, which the transport escalated to a session reset, and discovery trees + sharing-list reads then failed. - pl-client: distinguish a per-resource signature failure from a dead session by probing getSessionInfo before escalating; recognise the PlError family in isUnauthenticated; register sharing resource types (silence UNKNOWN RESOURCE TYPE); add the txListGrants:v1 capability; gate init role-resolution on publicGrants:v1; everyone-sentinel exact-token match. - pl-tree: discovery trees self-heal (re-discover + retry) when a discovered grant is revoked/expired instead of failing the whole ResourceTree poll. - pl-middle-layer: per-envelope grant reads with failure isolation; idempotent revokeShare. - pl-model-common: SharingOutbox / SharingState / SharedEnvelope type names.
do-pack produces package.tgz, so a second run saw two .tgz files and
`mv *.tgz package.tgz` failed ("dest is not a directory"). Remove the prior
package.tgz before packing. Fixed in the structurer template and propagated to
all blocks via refresh-blocks (only the do-pack line changed).
4 participants
Greptile Summary
This PR implements the "Copy & Share" project-sharing feature: donors can package one or more project snapshots into a
SharedEnveloperesource, grant it to named recipients or to everyone, and recipients can accept (copying the projects into their own list) or reject. The implementation spans the proto/gRPC layer,pl-client,pl-tree, andpl-middle-layer.ListUsers(recipient picker),ListGrants(donor's "who did I share with"),GrantAccess.MAKE_RESOURCE_PUBLIC, and three new capability tokens (crossTreeRefs:v1,userListing:v1,publicGrants:v1).AccessFlagsdefaults flip from deny-list to allow-list, and a newset_errorcontrol flag is added.PlTreeStateandSynchronizedTreeStateare generalized from a single root to aSet<SignedResourceId>, with a newSharedTypeSeedthat discovers shared envelopes via periodicListUserResourcespolling (paced at 1 in 15 iterations).PlTreeRootsEntry/PlTreeRootsAccessorexpose the reactive root set toComputables.SharingOutbox,SharingState, plus the shared-resource discovery tree), four public share-flow methods onMiddleLayer, a 6-hour envelope cleanup timer, and reactiveoutgoingShares/pendingSharescomputables.Confidence Score: 3/5
The sharing flow has two concrete defects in the share-creation and share-discovery paths that can produce orphaned envelopes and allow post-expiry acceptance of shares.
Two behavioral defects were found: (1)
shareProjectscreates a SharedEnvelope with no grants when called with an emptyrecipientsarray — the envelope lands in the donor's outbox and persists for 14 days, visible but unreachable by anyone; (2)createPendingSharesComputablehas no expiry check, so recipients see (and can accept) envelopes that have already passed their TTL, for up to 6 hours until the donor's cleanup timer fires. Both affect the primary sharing path and produce incorrect state visible to users.middle_layer.ts(shareProjects recipients guard) andsharing_list.ts(createPendingSharesComputable expiry filter) need fixes before the feature is production-ready.Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Donor as Donor (MiddleLayer) participant Backend as PL Backend participant Acceptor as Acceptor (MiddleLayer) Donor->>Backend: "withWriteTx(MLShareProjects)<br/>createEphemeral(SharedEnvelope, EnvelopeData)<br/>createField(outbox/{shareId}, envelope)<br/>grantAccess(envelope, recipient, writable:true)" Backend-->>Donor: tx committed Donor->>Backend: sharingOutboxTree.refreshState() Note over Acceptor: every ~3s (15 x 200ms) Acceptor->>Backend: "listSharedResourcesByType(SharedEnvelope)<br/>via ListUserResources" Backend-->>Acceptor: [envelope signed id] Acceptor->>Acceptor: "setRoots(discoveredEnvelopes)<br/>pendingShares recomputed" Acceptor->>Backend: "withWriteTx(MLAcceptShare)<br/>copyEnvelopeProjectsIntoList(envelope -> projectList)<br/>writeSharingDecision(SharingState, shareId, accepted)<br/>writeEnvelopeAcceptance(envelope, login, accepted)" Backend-->>Acceptor: tx committed Acceptor->>Backend: refreshState(projectList, sharingState) Note over Donor: every 6h Donor->>Backend: "withReadTx(MLEnvelopeCleanupScan)<br/>check expiresAt on each outbox envelope" Donor->>Backend: "withWriteTx(MLEnvelopeCleanup)<br/>removeField(outbox/{expiredShareId})<br/>backend auto-revokes all grants"%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Donor as Donor (MiddleLayer) participant Backend as PL Backend participant Acceptor as Acceptor (MiddleLayer) Donor->>Backend: "withWriteTx(MLShareProjects)<br/>createEphemeral(SharedEnvelope, EnvelopeData)<br/>createField(outbox/{shareId}, envelope)<br/>grantAccess(envelope, recipient, writable:true)" Backend-->>Donor: tx committed Donor->>Backend: sharingOutboxTree.refreshState() Note over Acceptor: every ~3s (15 x 200ms) Acceptor->>Backend: "listSharedResourcesByType(SharedEnvelope)<br/>via ListUserResources" Backend-->>Acceptor: [envelope signed id] Acceptor->>Acceptor: "setRoots(discoveredEnvelopes)<br/>pendingShares recomputed" Acceptor->>Backend: "withWriteTx(MLAcceptShare)<br/>copyEnvelopeProjectsIntoList(envelope -> projectList)<br/>writeSharingDecision(SharingState, shareId, accepted)<br/>writeEnvelopeAcceptance(envelope, login, accepted)" Backend-->>Acceptor: tx committed Acceptor->>Backend: refreshState(projectList, sharingState) Note over Donor: every 6h Donor->>Backend: "withReadTx(MLEnvelopeCleanupScan)<br/>check expiresAt on each outbox envelope" Donor->>Backend: "withWriteTx(MLEnvelopeCleanup)<br/>removeField(outbox/{expiredShareId})<br/>backend auto-revokes all grants"Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: Add revokeAccess method and enhanc..." | Re-trigger Greptile
Context used: