Conversation
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
from
September 3, 2026 22:56
24a03b3 to
7256fd6
Compare
Preview:
|
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
from
September 4, 2026 16:16
7256fd6 to
9ae99a3
Compare
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
from
September 8, 2026 17:37
9ae99a3 to
59d33f8
Compare
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
from
September 8, 2026 19:52
59d33f8 to
0bc4323
Compare
|
Posted 1 actionable inline finding. |
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
from
September 16, 2026 20:40
98d687a to
7b78ae3
Compare
|
Posted 2 actionable inline findings. |
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
2 times, most recently
from
September 17, 2026 00:05
6fec054 to
86814d2
Compare
Member
Author
|
@ask-bonk review this |
|
Posted 1 actionable inline finding. |
Confines the existing read-only GoogleDriveSession API to one selected folder's current descendant subtree, for both My Drive folders and ordinary subfolders inside a shared drive. A shared drive's own root stays with the Shared Drive resource. Drive v3 offers no folder corpus, no folder-scoped token, and no recursive ancestor predicate -- `'<id>' in parents` means direct children only -- so membership is proved in the gatekeeper. drive-folder-scope.ts walks each candidate's parent chain upward through freshly batched files.get metadata, level by level, admitting only acyclic same-domain chains of live folders that reach the root within Drive's 101-hop nesting limit, then re-reads every node on the surviving paths immediately before disclosure. Nothing is cached across the operation that proved it: a hierarchy change rotates no credential and bumps no cache generation. The grant identity is an internal `/_resource/folder/:folderId` path rather than the natural browser URL, because `/drive/folders/:driveId` is already the shared drive's permanent identity and its pattern leaves the query component wildcard -- a query-qualified variant would match both resources and make resource selection order-dependent. describe() still reports the natural URL for the UI. Folder-derived Doc and Sheet children now revalidate: DriveSessionCore .nativeRead() re-proves ancestry and the exact native MIME type before the provider is contacted, re-checks the proved chain before any approval, and discards the fetched value if either fails. Drive has no ancestry-plus-content transaction, so a move landing after that final check still returns; the next read denies, and the method says so. CursorPager's authorize() now takes `exhausted` and its page budget is maxProviderPagesPerCall: reaching the bound authorizes the empty nonterminal page and returns [] rather than throwing, so a folder cursor can filter a whole page without claiming there are no results. Only an exhausted cursor reaches the empty-search path, which now fences observer admission through ObserverTracker.prepareWithheld() -- a withheld read registers no tracked set, so addObserver would otherwise verify a candidate against nothing. The Drive observer denial no longer names a file ID.
startResourceConfigurator() previously returned only iframe HTML plus a gatekeeper-defined capability, so a gatekeeper had no way to say that selecting a resource first needs a wider grant on the connected account. The sandboxed frame cannot ask for one itself: it runs under sandbox="allow-scripts" with no popup or top-navigation privileges, and widening that sandbox would weaken every configurator to serve one. ResourceConfiguratorFrame now carries an optional authorization action: display copy plus a capability that prepares the consent attempt and returns its URL, or omits the URL when the authority is already held. The gatekeeper keeps the policy -- what scope, what nonce, what callback -- and Workshop contributes only the trusted user gesture. No arbitrary scope strings cross the boundary, and the field is optional, so every existing gatekeeper configurator is unaffected.
ResourceConfiguratorHost now wraps the sandboxed configurator and, when the frame declares one, renders the authorization control itself. The popup is opened blank during the click and navigated after the gatekeeper answers, because a window opened after an awaited RPC is blocked by default. popup.opener is cleared, and only an http(s) URL with no embedded credentials is navigated to; anything else closes the window and reports a failure rather than following it. Each action is keyed by frameKey, so switching account or resource while a request is in flight invalidates that request generation and closes its blank window -- a late answer cannot navigate a popup that now belongs to a different account or resource. Unmount does the same. A frame now owns two RPC capabilities, so disposeConfiguratorFrame() releases both and is used by both configurator owners on replacement, unmount, and late completion; the authorization stub is released even when disposing the UI stub throws. ResourceConfiguratorHost.test.tsx covers the blocked popup, the valid and credential-bearing URLs, the already-granted answer, the fenced late result, and disposal.
One Google Drive Folder resource now covers both an ordinary folder and a Workspace Shared Drive's root, so the separate Shared Drive resource and its picker are gone. The identity is the natural browser URL, https://drive.google.com/drive/folders/<folderId>, which is unambiguous once no second resource claims that path, and the folder's grant stays the narrow drive.metadata.readonly, documents.readonly and spreadsheets.readonly set. The capability is positioned rather than recursive. GoogleDriveFolderSession lists and searches only the current folder's direct children, and openFolder() appends one validated direct-child edge to a new, independently disposable capability, leaving the parent untouched. Both list() and search() carry a direct-parent predicate into the provider request -- corpora=drive for a shared-drive root, corpora=user otherwise -- so Google's own full-text index answers within the folder instead of a broad search being filtered afterwards. That replaces the upward ancestry proofs: every operation revalidates the root and the root-to-current path, and observations are typed as files or listable folders so a remembered folder unit must still be a live listable folder. Complete shared-drive discovery cannot be narrowed -- Google accepts nothing smaller than drive.readonly for drives.list -- so it is an optional, user-initiated expansion on the account rather than part of any folder's grant. The stored OAuth flow carries a requestDriveReadonly flag, which adds only that scope and is never recorded as resource consent; a reconnect preserves an expansion the account already made. listSharedDrives() checks the stored scope before calling the provider, and the picker's Workspace Shared Drives source is offered alongside Folders, which lists every folder the account can list children of, including ones another person shared with it.
A folder session refused every out-of-scope direct lookup through #authorizeWithheld, which commits the observer-withheld marker and makes every future addObserver() fail. The trigger was ordinary use rather than probing: a user pastes a link to a doc that lives one subfolder down, the agent opens it by ID, and the binding is silently unshareable from then on with no way to clear it. Only one of the two refusals answers an owner-relative question. #tryFetchFile collapses 403 and 404 to undefined, so an absent file means "this account cannot see it" -- a negative a collaborator might not share, which stays fenced. Whether a visible file is a direct child of the positioned folder is objective, so refusing it discloses nothing and now leaves admission open. Both paths still raise the same outsideScope() error, so the caller cannot tell them apart. The move-during-read test correspondingly asserts the stronger claim it was always about: the moved file stays visible, so the content reaches neither the approval queue nor the caller and nothing is authorized.
The trusted authorization control owns popup state, an unmount cleanup and the whole OAuth hand-off, which the frontend conventions say earns a file of its own; it sat inline in the configurator host. Its tests move with it, leaving the host suite the cases that are about host wiring. Its popup now opens under a stable window name. The account holds a single pending OAuth flow, so a second click used to open a second tab and strand the first on a superseded nonce; naming the window makes the repeat click renavigate the same tab instead. Polling popup.closed to re-enable the button was the alternative, but a background tab left open would then wedge the control disabled long after its flow died. Both hosts clear a superseded frame from an effect, so one committed render could pair a newly chosen account with the previous account's frame, and acting on it would configure or authorize the wrong account. The host now takes the frame state plus the current selection and resolves them through one guard, so no call site can forget the check. GatekeeperModal already made the comparison at submit time in two places; both route through the same helper now, and its duplicate local state type is gone. BlueprintLandingPage records the account and resource pattern its frame state never carried.
DriveFolderSessionCore was added as a copy of DriveSessionCore, leaving two implementations of the staging, fencing and page-authorization rules. They had already drifted: #authorizeFiles never discarded a staged check when the approval was refused while its twin did, which is harmless only because prepareObservation returns no discard today. Both cores now extend DriveCoreBase, which owns authorizeUnits, authorizeWithheld, tryFetchFile and the per-page rule, parameterized by the revalidation and base units a moving scope adds. The empty-search refusal has one definition rather than two that a reader has to diff by eye. Several facts had spread across layers on the way in. The live-listable folder triple was written out six times in three files, and two of those asserted only two of its three conditions; it is now isListableFolderNode and isListableFolderFile beside the MIME constant that defines it. The 403-or-404 "invisible to this account" rule collapses into isFileInvisible, so the single-get paths cannot diverge from what the batch path treats as account-wide. checkObservations carried a second hand-written field mask feeding the same parser and now sends DRIVE_SCOPE_NODE_FIELDS. oauthScopeCovers spelled its scope implications as branches over inline literals, where a typo reads as an ungranted resource and silently hides a configurator; it is a SCOPE_COVERED_BY table so the next readonly pair is a row. Four pieces of state or code turned out unreachable. driveFileToEntry took a rootId no call site passed, so its documented rule withholding the bound folder's own parent never ran. DriveApi.getDrive lost both production callers when folders and shared drives merged. FolderLocation.rootId is always folderIds[0] and was validated to be, so it was an invariant to maintain rather than a fact to store. The optional NativeRead parameters let a future opener construct a Drive-derived session that silently skips revalidation, so they are required now and the one binding that genuinely has nothing to revalidate says so. The rest is local. GoogleDriveSessionImpl had three copies of dup-the-queue-and-dispose-on-throw and recovered folder-ness with an instanceof after building a core to ask; one #withQueue and the scope kind it was re-deriving replace both. DriveSessionCore.nativeRead captured this to reach one field, pinning the core's api, scope and observer hooks for a native session's whole life, and now passes the function. The trusted authorization control tracked one in-flight request through two refs that had to agree on four exit paths, and told every gatekeeper's user to retry a shared-drive selector.
The folder picker had two sources. **Folders** searched `corpora=user`; **Workspace Shared Drives** enumerated `drives.list`, which Google offers under no scope narrower than account-wide `drive.readonly` -- so listing shared-drive roots cost an optional account-level expansion, carried by a trusted authorization card outside the iframe and a whole reconnect flow whose only purpose was widening that one grant. Search `corpora=allDrives` instead. It spans My Drive, "Shared with me", and every shared drive the account is a member of, and `files.list` accepts it under the `drive.metadata.readonly` the folder resource already requires. One field, one request, no second consent, and a shared-drive root still reachable when Google returns it as a folder. What that buys is worth stating plainly: the picker is now an interactive search over one provider page, not an enumeration. It never promised completeness in a useful sense anyway -- the old Folders source was already one page -- and a known root that does not surface is still connectable through the concrete folder URL, which opens the selector prefilled. So no traversal pager, and no silent bounded collection pretending to be exhaustive. `allDrives` introduces one failure the single-corpus searches could not have: Drive may answer `incompleteSearch: true`, a partial result that is otherwise indistinguishable from a complete one. Reject it at the provider boundary rather than inventing a warning envelope no caller would be required to read. A normal `nextPageToken` is not that, and must not be mistaken for it. Positioned folder sessions are untouched: a bound shared-drive folder still navigates its own `corpora=drive`, now pinned by a test, so discovery cannot leak into a binding's boundary. `drive.readonly` survives only as a private row in `SCOPE_COVERED_BY`, where it truthfully subsumes the narrow requirements an account may already hold more than. Nothing requests it.
Nothing needs it any more. The Google folder picker was its only producer, and it now finds shared-drive folders under the scopes its resource already requires, so the whole vertical slice -- an optional `ResourceConfiguratorAuthorization` on the frame, an optional `GatekeeperUser.requestResourceAuthorization()`, and the trusted frontend control that ran it as a nonce-carrying connect flow -- is authority with no caller. Kernel surface that exists for one deleted feature is surface that will be reached for by the next one. This also reverts the host refactor that came with it: frame identity and disposal move back to each owner, where they were before. Keeping the refactor would leave `GatekeeperModal`, `BlueprintLandingPage`, and `ResourceConfiguratorHost` rewritten in a change that no longer has any reason to touch them, and the pre-existing per-owner behaviour is sufficient for the surface that remains. `ResourceConfiguratorFrame` keeps its name and returns to being the plain `GatekeeperUiFrame` alias every gatekeeper already references. `workshop-shared`, `workshop-backend`, and `workshop-frontend` are now identical to the merge base, so the Drive folder feature carries no external-package diff at all. The normal resource-consent path -- `ensureResources`, `reconnectAccount`, staged restore/commit, `completeConnectHandoff` -- is untouched throughout.
`prepareWithheld` writes its in-flight marker before asking for approval precisely so an activation that dies mid-read leaves admission closed rather than open. The catch in `authorizeWithheld` then discarded that marker for *any* `authorizeObservation` failure, which undoes the thing the ordering was for: the overseer can record the observation and lose the response, and a deleted marker lets `addObserver` admit a later collaborator with nothing to verify them against. Only a marked refusal proves nothing was recorded. Everything else is an unknown outcome, and an unknown outcome reaches the same durable state as a commit -- the fence stands for good. That is already the rule in `gatekeeper-kit`, where `prepareWithheld` gives `commit` and `abandon` the same implementation and reclaims state only for `isObservationRefused`, so reuse the helper rather than keep a second answer to one question. Worth knowing while reading this: nothing marks refusals yet, so today every failure takes the unknown path and `discard` is unreachable. That is the fail-closed direction, and the branch is here so it stays correct when the overseer starts marking them.
`#cursor` revalidated the saved root-to-current path before returning the pager, to learn which corpus to search. Nothing had authorized a read at that point, so `list()` and `search()` answered whether the path is still visible, listable and connected without an approval-queue entry or any observer bookkeeping -- calling either and never paging was enough, and repeating it is an unlogged change monitor over the bound folder. Every other method on this session authorizes before it returns, and `#requireDirectFile` fences an invisible probe outright, so this was the one path that read first and audited later. `fetchPage` already revalidates the path before each page, so take the corpus from that read instead and keep the cursor synchronous, as its account-scoped sibling has always been. The moved-drive check needs a baseline rather than a pre-read: the first revalidation records the drive it found and every later one compares against it, which is the same guarantee one page later.
`#assertParent` used to refuse only a non-folder, which is objective: any
observer who can read the file's metadata can reach the same answer, so
recording a plain file unit was honest. This branch added
`canListChildren !== true` to the same condition without changing the
unit, and that half is owner-relative. A file observation proves metadata
access alone, so a collaborator who can see the folder but cannot list it
passed the backward check vacuously -- inheriting either the owner's
successful cursor or the refusal, on a folder they have no right to
enumerate.
Take the shape `openFolder` already uses for the identical predicate: a
live folder this account cannot list is owner-relative and takes the
withheld fence; a non-folder or a trashed one stays an objective file
disclosure, so a probe of some unrelated ID cannot latch sharing closed;
and a parent that passes is recorded as `{kind: "folder"}`, which
`checkObservations` verifies with `isListableFolderNode` rather than bare
reachability. That last part also covers the empty-listing case, where
the page observation carries no unit of its own.
Folding the trashed check in comes free with `isListableFolderFile`, and
matches the root and direct-child validators.
`normalizeSearch` throws "Drive search requires at least one filter", and
this branch condensed the sentence that said so out of the agent-facing
docs. Every `DriveSearchQuery` field is optional, so `search({})` and
`search({order: "nameAsc"})` type-check and then fail at runtime -- and
an order-only call is the obvious way to ask for a sorted listing, so an
agent reads the types, writes it, and burns a turn on the throw. The
sibling constraint on `order` survived on its own field; only this one
went.
`drive-types.txt` is what `getTypeScriptTypes()` ships, so the JSDoc here
is the whole contract an agent gets. Restore the requirement, restore the
note that omitting `order` keeps Drive's relevance ordering for full-text
search, and state both on the folder session too: it runs the same
`normalizeSearch` and withholds the same empty result, and documented
neither.
`GoogleDriveSessionImpl` implements both Drive session interfaces, so the
signatures `@validateRpc()` builds validators from are the account ones:
`DriveListOptions` and `DriveSearchQuery`, both carrying
`directParentId`. A folder capability advertises
`GoogleDriveFolderSession`, whose query types omit that field, and got
handed it anyway.
`list` discarded it. `search` did not: it satisfied `normalizeSearch`'s
required-filter check, so `search({directParentId: "decoy"})` ran where
`search({})` correctly throws, and while `#cursor` overrides the field
for the provider call, the same object reaches `pageAuthorizer` and
`queryClauses` renders `parent decoy` into the approval description --
indistinguishable from the genuine parent filter an account-scope search
shows there. The units and the provider query stayed correct, so nothing
escaped the folder; a gadget could put a chosen id into its own audit
line.
Take the folder query types on the folder core's own methods and refuse a
`directParentId` that arrives regardless. Refusing rather than stripping:
the published type says the field does not exist, so a call carrying it
is invalid, and silently dropping invalid input is how `list` hid this in
the first place. Separately validated session classes would make the
validated surface exact without a guard, but that means a facade per core
and widening the Durable Object's declared `Gatekeeper<GoogleDriveSession>`
to a union -- more wire surface churn than this earns.
`#exactFileCursor` drops a trashed bound file, so `list()` drains to `null` on an exact-file binding whose file has been binned -- while `getEntry()` still returns it, as its own doc says. The behaviour is deliberate and pinned by two tests, and it is the same rule `buildDriveQuery` applies to every other listing through its constant `trashed = false`. What was wrong is the exported prose: this branch condensed away the paragraph that used to state it, leaving `list()` promising "the one exact-file entry" flatly, with the escape hatch documented on one side only. That makes three runtime constraints lost to the same condensation, after the search filter requirement and the relevance-order note. So restore the canonical statement on `DriveEntry`, where it covers both sessions at once, and take `list()`'s undocumented throw with it -- `directParentId` has to name a folder this account can list, and the refusal moved twice on this branch without ever being written down. I checked the rest of what the condensation dropped rather than assume: the cursor's `[]`-versus-`null` contract survived and reads better than it did, `fullTextContains`'s reach and `getEntry`'s bound-ID restriction are intact, and the shared-drive scope paragraph went with the feature it described. Two items, not ten. Also note the one Drive URL shape that cannot be connected, since the failure is a bare 404 at connect time with nothing to read it against: `?resourcekey=` is dropped, which only matters for a link-access user of an item shared by link before September 2021. Support means carrying per-file keys through every request including batch subrequests, so it is its own change; the README now says so rather than leaving the 404 unexplained.
A folder inside a shared drive can be shared directly with someone who is not a member of that drive -- Google documents this as a file access permission, deliberately contrasted with member permissions, and parent permissions propagate down to children. Such a folder passes `readFolderRoot`: `files.get` reports its `driveId` and `canListChildren: true`, so the binding connects. Then the folder cursor sends `corpora=drive&driveId=...`, which is an access of the shared drive itself, and Drive answers 403 `teamDriveMembershipRequired`. Browsing is dead while `getEntry`, `openFolder` and native opens keep working, since those are all `files.get`. The refusal is loud, which is the one mercy here: `fetchPage` sits outside `#nextPage`'s only try, so the error lands before `buildEntries` and before `authorize` -- nothing is observed and the withheld fence is not latched. This commit does not fix it, it stops the failure being a bare `403`. Switching to the user corpus is the obvious-looking repair and it is a trap: Google defines that corpus as My Drive items the account created or opened plus items shared directly with it in "Shared with me", with no shared-drive clause at all, so a *member* browsing a team folder would lose the listing entirely, and a non-member's children are inherited rather than directly shared, so they likely would not appear either. An empty page is not inert on this path -- `search()` runs with `denyEmptySearch`, so it latches the observer fence for good. `allDrives` is no better: it too is scoped to drives the user is a member of, and it is the corpus Google names as the source of `incompleteSearch`, which this client hard-fails. So the real repair is to refuse at connect, or to detect membership, and it wants a live non-member account to design against. Until then the error says which drive fact is missing, and the README stops implying the corpus rule always works. Its account-binding paragraph also repeated a looser claim about `corpora=user` than Google's own definition supports; that now matches the documented wording.
`#getSnapshot` published its promise to `#snapshot` synchronously, and the folder binding's scope check straddles the fetch it wraps: precheck, fetch, postcheck. So a read refused by the postcheck still left its revision in the cache, and `#nextSnapshot` returns a cached revision inside `DOC_SNAPSHOT_TTL_MS` without contacting Docs at all. Move a document out between the precheck and the fetch, edit it, move it back inside ten seconds, and the retry is served the revision captured while the document was outside -- certified as an in-scope read by an approval record that says so. The README already promised the opposite: "if the file moves meanwhile, the fetched value is discarded." Fold both snapshot callers into one `#readSnapshot` that owns the rollback, so there is a single place where a refused read and its cache entry are reconciled. An identity check guards the rollback, leaving alone a snapshot a concurrent read has already replaced, and any failure reclaims it rather than only a membership one -- the distinction buys nothing and an unrelated refusal costs one refetch. Two properties this had to keep, and the first one I broke on the way past. `NativeRead` takes a thunk precisely so the precheck can refuse before the provider is contacted, and starting the snapshot chain eagerly defeated that: every refused read went on to hit the Docs API, which `refuses every Doc read after the document leaves the subtree` caught. The chain now starts inside the thunk. Second, the assignment stays synchronous within that thunk, so concurrent reads still chain onto one fetch instead of racing to overwrite each other. Scope is folder-derived Doc sessions alone: the editable Doc session and every account or exact-file binding read through `unguardedNativeRead` and have no postcheck to fail, and the Sheets session caches nothing.
Trimming the supplied parent brought `list()` in line with `buildDriveQuery` and `normalizeSearch`, and took a refusal with it. At merge base a whitespace id was truthy, so it reached `#assertParent`, `files.get` answered 404, and the call failed. Now it trims to empty, skips the check and the parent predicate, and returns an account-wide listing: the caller asked to narrow to one folder and silently got everything. `search()` has always done this when another filter carried the required-filter check, so that half is not a regression, only the same mistake. Nothing escapes the binding -- an account binding may list its account, and the approval description says "the connected Drive account" with no parent clause, so the audit is accurate about what happened. The damage is downstream, in an agent reasoning over the whole account as though it were one folder's children. Hence a refusal rather than a wider trim. A blank name or full-text filter matches more rows within the scope the caller already has; a blank parent returns rows from outside the folder they named, which is a different kind of mistake and the reason this one field gets the check. Both readers share it, since `normalizeSearch` is where the search path trims.
Rolling the snapshot back after a refusal cannot work, and the identity check I added made that look handled. A read that chained onto the refused revision already holds it: `#nextSnapshot` returns the same object inside the TTL, and the refused fetch itself resolved -- only the guard around it failed -- so the sibling read publishes its own promise, the identity check no longer matches, and the revision is disclosed by whichever read passes its own postcheck. Tainting the revision is equally too late, for the same reason: by the time the refusal is known, the sibling is past its fetch. And publishing only on success is not available -- `fetches the document once for concurrent reads` pins that reads issued together share one revision, because otherwise they observe different documents and the later response can be the older one. So split the one field by what it is for. `#inFlight` shares a fetch that is still running, which is what that test is about and is safe on its own terms: each joiner brackets the shared fetch with its own checks, exactly as a lone read does. `#approved` holds only a revision a completed check approved, and is the only thing a later read may reuse. A refused read assigns nothing, so there is no state left to roll back, taint, or reason about. `#nextSnapshot` now reads a plain value and loses its await-and-swallow. The precise interleaving is closed by construction rather than by a test: reproducing it needs a move landing between two postchecks, and the provider stub has no seam for ordering Drive reads against each other. The new test locks what is checkable -- reads sharing one fetch, refused together, leave nothing reusable -- and passes before this change too, so it is a guard against regression rather than proof of the fix.
ndisidore
force-pushed
the
feat/gk-google-drive-folder
branch
from
September 18, 2026 14:40
662969c to
644e01d
Compare
|
Posted 3 actionable inline findings. |
|
`#inFlight` clears when the fetch settles, not when the guard around it finishes, so a read whose postcheck is still outstanding can be overtaken: a later read fetches a newer revision, publishes it, and the slow one then publishes its older revision over the top. Every subsequent call reuses the older one for the rest of the TTL, so a caller that already saw the newer content can watch an edit disappear. Nothing leaves the binding -- both revisions passed complete guard cycles -- and the overtaking needs the slow read's remaining postcheck and authorize to outlast the other read's entire cycle, which is more work. It is still the property `fetches the document once for concurrent reads` exists to protect, arriving through the sequential path instead of the concurrent one, so publish only what is at least as recently confirmed as what is already there. A guard on publication rather than serializing the whole guarded read, which would reintroduce the head-of-line blocking the shared fetch exists to avoid. `>=` so a shared fetch, where both reads hold the same object, still publishes as a no-op.
The folder picker rendered name plus one of "In a shared drive", the owner's name, or "My Drive". Two folders called Engineering in different shared drives are byte-identical rows that mint different capabilities, and duplicate names across drives are ordinary rather than unlucky. This lands at the consent step, where the user is deciding what authority to hand over, and the mistake never surfaces afterwards either: `describe()` reports the folder's name, so the Connections list shows the same label for whichever one got bound. The file picker collides the same way when two files share a name, type and modified date. Fill the `meta` slot, which is already rendered and already carries `accessRole` for Calendar and `location`/`type` for BigQuery in this same file -- the Drive pickers were the ones leaving it empty. A tail of the ID rather than the whole thing, because `meta` does not shrink and a full ID would push the subtitle into an ellipsis, trading the useful column for the disambiguating one; eight characters separate anything inside one page and still match the tail of the ID in a Drive URL. This makes a collision visible and checkable, not resolvable: it says the two rows differ, not which one the user wants. Labelling by containing drive or parent folder would, and needs either the `/drives` surface this branch removed or a batched name lookup per search, so it is its own change.
|
Posted 1 actionable inline finding. |
The folder `getEntry` converted the file and authorized afterwards, so a conversion failure answered whether an id is a live direct child without the file being recorded for a later observer to be checked against. Its account sibling already authorizes the raw id first; this is the same ordering, and the description uses only `file.id` either way, so nothing is lost by moving it. No caller can provoke it. `driveFileToEntry` fails only when Drive omits `mimeType` or `modifiedTime`, sends a `size` that is not a safe non-negative integer, or ships a shortcut with no `targetId` -- every one a violation of the response shape Drive documents and this client explicitly requests. So this is defensive ordering rather than a closed leak, which is also why it is worth the two lines: it costs nothing and removes the one site where the fix needed no judgement. The paging conversions at the other three call sites are deliberately untouched. Under the taxonomy those failures are provider outages, not facts about a file's relationship to the binding, and an outage recorded as a scope fact is what `isAccountWide` exists to prevent -- so the expected outcome there is a comment, not a `CursorPager` contract change. That belongs with the two other findings waiting on the same taxonomy, and on the prior question of whether a bound path's ancestors are always covered by recorded folder units, which decides whether a path failure fences or merely observes. Written up in full outside the repo.
|
LGTM! |
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
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.
Replaces the separate Shared Drive resource with one Google Drive Folder resource, read-only.
One selection covers an ordinary folder in My Drive, a folder someone else shared with you,
or a subfolder inside a shared drive.
A folder binding sees its direct children only, not everything beneath it: listing and
provider-side search are scoped to that one folder, and a child folder is reached by opening
it as its own capability rather than by recursive traversal.
It requests the same three read-only scopes as the account and exact-file resources. The picker
is one
corpora=allDrivessearch, which finds every folder kind above without an account-widegrant. It returns one provider page of suggestions, not a complete list, so a shared drive's own
root or a folder that page misses is still connectable by its URL, which prefills the form. An
incomplete search from Drive fails rather than looking complete.
Folder-derived Doc and Sheet sessions re-prove ancestry on every method call, not only
at open, so a file moved out of the folder stops answering through a session that is
already open. Drive has no ancestry-plus-content transaction, so a move landing after
the final check still returns and the next read denies.