🔧 PUT-1585: Share flag in readdir shared users in stat - #3644
Merged
jfcastro92 merged 17 commits intoAug 26, 2026
Merged
Conversation
Directory listings need a shared/not-shared flag per entry. Ask the share index once per batch, chunked, rather than a query per entry, and answer only for entries the caller owns so listing a foreign directory never queries at all. Direct shares only — inherited access stays with listSharesOf, which already reports it.
isShared on /fs/stat and /fs/readdir, is_shared on their legacy counterparts, and return_shares now fills the shares array the legacy stat has been stubbing with []. Null for entries the caller does not own, so a share recipient is never told who else can reach the owner's files. The share-to-wire mapper moves out of ShareController so both controllers publish one shape.
Carry the flag into the v1 entry shape and add returnShares to stat(), whose shares are mapped into the same Share objects getShares() returns. returnShares joins the dedupe key and bypasses the entry cache in both directions — a result carrying share data is never written to it, so a later plain stat cannot serve other people's names from cache.
Around 500 tests run through one seeded free-tier account, so the per-tier windows were throttling the suite rather than anything it tests — adding a single readdir was enough to trip fs:readdir-burst. Resolve the seeded users to the unlimited policy, alongside the subscription gate the harness already turns off for the same reason. Test config only; no published limit changes.
jfcastro92
changed the title
Contributor
Coverage Report for puter.js SDK
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
revoke-user-user withdraws a grant without touching the share index, so the row outlived the access — invisible until now, because listSharesOf filters against live grants, but the new flag reads the index and would report a file as shared to nobody, permanently. Drop the row where the grant goes. The alternative, filtering liveness on the read side, is the per-entry work the flag exists to avoid.
Nothing invalidated the SDK entry cache on share or unshare — the socket handlers only fire on item mutations — so is_shared, which now rides in the cached entry, stayed stale for every consistency: 'eventual' read. That includes the GUI's own listing refresh, which defaults to it, so a badge would not have appeared until an unrelated write flushed the cache.
stat() and readdir() return FSItemRead, so the is_shared the docs lead with typechecks for TypeScript consumers rather than erroring on FSItem. The docs said "you have shared", but the query has no issuer predicate: a manage delegate's re-share sets the owner's flag too, which is the useful answer and matches getShares().
A shared file looked exactly like a regular one. The data to tell them apart arrived with the readdir/stat share flag; nothing rendered it. Adds the badge to both listings — UIItem (desktop, explorer windows, file dialogs) and the dashboard's Files rows — fed from is_shared, and keeps it in step with the share dialogs: both funnel every grant, mode change and revoke through one render, so the badge follows without waiting for a re-listing. Inherited access is deliberately not badged. It is a state of the folder that was shared, so marking every file inside would repeat one fact on hundreds of items; the backend flag is direct-only for the same reason. The icon (owner-shared.svg) and the strings (item_shared_by_you, in 40 locales) were already in the tree, unused — only the wiring was missing. The blue ring is doing the work: list view shrinks badges to 8px, where a glyph is illegible and the white circle the sibling badges use disappears into the row.
Review feedback: too big, and in the wrong place. It sat in the badge cluster, which the dashboard pins to the row's top-left corner rather than to the icon — and at 12px it dominated a 24px row icon. Now a dot on the icon itself, lower-right: 9px on the desktop's 45px icons, 7px on the dashboard's 24px rows. Anchoring to .item-icon rather than to the badge cluster is what keeps it on the corner at both sizes. Dropping the people glyph with it — unreadable at either size, and colour was the signal the ticket asked for.
It sat on the icon box's corner, which is 5px outside the artwork on every side, so the dot read as clipped — half of it hanging over empty padding with the icon's own drop-shadow falling across it. Nudged in on both axes: 7px on the desktop, 2px on the dashboard rows.
A recipient given "can edit & share" could not pass that level on: the dialog offered it, the server refused it, and the refusal was a bare 403 Forbidden that reads as a bug. Handing out manage needs authority over manage, which only the owner has — the refusal is right, the dead end and the silence were not. The dropdown now withholds it from anyone who does not own the item; a row already set to it keeps it, so opening the dialog cannot downgrade the owner's own grant, and a mixed selection follows its strictest item. The server says why, and only to someone who can already share the item — a stranger still gets the ACL's own safe error, which does not admit the node exists. Verified against a running server: a delegate grants read and write as before, and manage now answers cannot_delegate_manage with a sentence naming the owner as the one who can.
Sharing a file with someone who already had it answered "Shared with X", the same as a first share, so the dialog claimed to have done something it had not. The service already knew — it computes isNew to decide whether to notify the recipient — but the flag stopped at the controller. It now travels on share results only; a listing describes standing access and says nothing about it. Which of the three things happened is settled client-side, since the mode each recipient holds is already on screen: created access, raised or lowered it, or changed nothing. That keeps the previous mode off the wire, and an older backend that omits isNew still reads as a share, which is what these dialogs said before.
Salazareo
reviewed
Aug 26, 2026
…ared-users-in-stat
…-dialog-correctness 🐛 PUT-1586 + PUT-1599: Share dialog says what it can grant, and what it did
…n-readdir-shared-users-in-stat' into juancastro/put-1590-sharing-shared-files-are-not-different-than-regular-files # Conflicts: # src/gui/src/UI/Dashboard/UIShareModal.js # src/gui/src/UI/UIWindowShare.js
…ared-files-are-not-different-than-regular-files 🔧 PUT-1590: Mark shared items in the file listings
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
2 participants
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 this adds
A file's listing entry now says whether it has been shared, and
stat()can name who it was shared with.isSharedon/fs/statand every branch of/fs/readdir;is_sharedon their legacy counterparts and on the v1 entry shape the SDK publishes.return_sharesonstatfills asharesarray with the same objectsgetShares()returns — inherited access and unclaimed invitations included. The legacy/stathas accepted this flag and answeredshares: []for as long as it has existed; it now answers for real.is_sharedon stat/readdir results,returnSharesonstat(), both typed.The performance compromise
The ticket asks whether children of a shared folder should be flagged, and whether that can be fetched without hurting readdir. The answer here is direct shares only:
SELECT DISTINCT fsentry_id … IN (…)onidx_share_fsentry— never a query per entry, and it runs alongside the suggested-apps and thumbnail work the listing already does rather than after it.false, andgetShares()/stat({ returnShares: true })remain the place inherited access shows up, viainheritedFrom.Permissions callout
isSharedis owner-scoped. For an entry the caller doesn't own it isnull, notfalse— a share recipient browsing an owner's folder is never told whether other people can also reach those files.sharesis gated onmanage, the same authorityGET /share/sharesrequires. Without it the array is empty rather than an error, so asking for it can't turn astat()you were allowed to make into a failure.manage, not only the owner's own — the same setgetShares()reports. That is deliberate, and the docs say so.Reading the index required making the index honest
listSharesOffilters its rows against live grants, because asharerow can outlive the grant it records. A listing can't afford that filter — it is exactly the per-entry work this design avoids — so the row itself has to be truthful.One route left it untruthful: the deprecated
POST /auth/revoke-user-userwithdraws a grant without touching the index. Reproduced directly —isShared: truealongsidelistSharesOf: [], i.e. a file reporting itself as shared to nobody, permanently. The route now retires the index row with the grant (ShareService.onGrantRevoked), with a regression test. Two other paths I suspected turned out to be clean already: revoking a delegate, and a delegate leaving, both delete their rows today, which I verified rather than assumed.Why both API versions changed
puter.fs.stat()calls the legacyPOST /stat, not/fs/stat. Only touching the v2 controller would have shipped a flag no SDK caller could see, soLegacyFSController.stat/readdirandtoLegacyEntrycarry it too. Where the two disagreed I matched v1:sharesis always an array when requested, since that is the shape the legacy route has been returning.ShareController#toClientSharemoved tocontrollers/share/clientShare.tsso the FS controllers publish the identical share shape instead of growing a second copy that could drift.One SDK bug this surfaced
Nothing invalidated the puter.js entry cache on
share()/unshare()— the socket handlers only fire on item mutations.is_sharedrides in that cached entry, so everyconsistency: 'eventual'read kept serving the pre-share value, and the GUI's own listing refresh defaults to eventual. The badge this feature exists to drive would not have appeared until an unrelated write flushed the cache. Both operations now drop the affecteditem:/readdir:keys; the tests fail without the fix.Verification
canReadconfirms the recipient can reach it; the index/grant agreement above), and real HTTP for both API versions.check:puterjs:typesclean; docs build clean.Notes for reviewers
Give the puter.js API suites rate-limit headroomis separable. ~500 API-suite tests run through one seeded free-tier account, and adding a singlereaddirwas enough to tripfs:readdir-burston the browser runner. It resolves the seeded users to the unlimited policy, next to the subscription gate the harness already disables for the same reason. Test config only — no published limit moved, sorate-limits-and-quotas.mdis untouched. The cost is that no api suite exercises free-tier windows any more. Drop that commit and the suite's readdir assertion if you'd rather keep them; the backend HTTP tests and the mapper unit test still cover the flag.return_sharesruns the fulllistSharesOfunderFS_STAT_LIMIT(1200/min) rather thanSHARE_LIST_LIMIT(600/min), and the two counters are separate, so a caller can do that work faster throughstatthan through the route built for it. Consuming a second scope conditionally isn't expressible in the current gate — worth a follow-up rather than something improvised here.