Fix/workspace share resolution - #152
Open
hmahmood24 wants to merge 5 commits into
Open
Conversation
A provider sharing link resolves through /shares/{token}/driveItem, and
the proxy broke that route four separate ways. Share tails were fed to
the drive-tail classifier, but a share's driveItem is an item, not a
drive root -- so /content (the canonical shared-file download) and the
colon path forms classified unknown and 403'd, while /children
allow-checks built /driveItem/root, a segment beneath an item that Graph
rejects, turning every share listing into an upstream 400. Share tails
now classify as item subresources anchored on the driveItem, and the
ancestry layer maps a share base's root anchor onto the driveItem-
relative URL shapes.
Redirects were dead ends. Graph answers share-token requests with a 308
the sandbox cannot follow -- it holds the proxy nonce, not a provider
token -- and the ancestry clients did not follow redirects either. Share
reads now follow redirects inside the proxy, where the real token lives
(httpx drops it on cross-origin hops by itself); a Location that still
reaches the sandbox is rewritten to the proxy root, where the nonce
works; and a redirect the client cannot follow surfaces as a 502 naming
the failure instead of an opaque proxy 500. This is the observed staging
failure: an authenticated /shares request died as a bare 308 with
nothing anyone could act on.
Nothing redeemed the link. An anyone-with-the-link or cross-tenant share
is unusable through /shares until the recipient redeems it -- the API
equivalent of opening it in a browser -- and such shares never appear in
sharedWithMe, so the item is invisible everywhere until then. The proxy
now sends Prefer: redeemSharingLinkIfNecessary on shares requests; the
caller handed the assistant the link, so redemption is the intent, and
IfNecessary keeps it a no-op where access already exists. A caller-
supplied Prefer wins.
…orkspace
The proxy could not serve the shares surface, and nothing told the actor
it existed -- so faced with a SharePoint sharing URL it scraped the share
page's HTML and extracted page-embedded grants, exactly the fragile,
credential-leaking route the workspace connection exists to replace.
Three additions, one per layer. The prompt's OAuth section gains the
recipe: encode the URL into a u! share token, walk /shares/{token}/
driveItem through the proxy, or re-address via /drives/{driveId}/items/
{id} after the first resolution -- never fetch the link itself. A
platform/workspace-sharing-links builtin guidance entry carries the same
procedure with the failure modes (absence from sharedWithMe says nothing
about access; older runtimes need the Prefer header set by hand) to
every assistant through the hash-guarded platform catalogue. And
get_oauth_prompt_context now names which providers actually hold a live
connection, read from the runtime secret store without a network call --
plans stop guessing whether the workspace works and stop asking the user
to reconnect accounts that already do.
A provider app whose sync succeeded with zero rows reported plain "ready", which reads as usable and sends the actor searching for tools that do not exist. The zero-tool case now names the condition and the remedy -- likely missing credentials or scopes, reconnect the app -- so the prompt line and Console stop telling different stories.
… the run Three defects in the two-phase submit, each found by walking the bulk path end to end rather than by a test failing. Source files were read wholly into memory to be uploaded -- a 346 MB CSV cost 346 MB of pod RAM inside the very boundary the dispatch tier exists to protect, and it is exactly the multi-hundred-MB files that dispatch. Uploads now stream from disk. The stage-upload-publish sequence ran synchronously inside submit(), holding the caller's thread for as long as the uplink takes while the prompt promises a handle immediately. Dispatch now runs on the manager's pool, like inline execution, and its failures land on the run row like every other outcome. A dispatch that raised -- or a pod that died -- between the run-row insert and the publish left the run queued forever, the one state whose next step is "keep polling", for a run no worker will ever pick up. The raise now records a failure; the pod-death case is settled at read time, deterministically: uploads only ever run in the submitting process, so a dispatched row with no dispatch id and no upload in flight in this process has no future. No timers, no grace periods. A request whose source stages no files is refused before the publish, because a zero-job dispatch succeeds at every step and then folds to queued eternally.
…xecute them Dispatching a RowsSource or TableSource above MAX_INLINE_ROWS published zero jobs and hung forever. The fleet's unit of work is a staged file -- publish_submit emits one parse message per uploaded source, and a rows or table request stages none -- so the dispatch wrote a manifest with an empty job list and its status folded to queued eternally, unreachable by every retry scope. This is precisely the bulk case the tier existed for: any connected-app pull over ten thousand rows, with a fleet reachable. It was never seen live only because the fleet was invisible to assistants until the control-plane env fix landed. choose_tier now keeps rows and tables in process whatever their size. The inline engine checkpoints, verifies against the declared count and resumes, so size costs the assistant latency and contention rather than correctness -- against an eternal hang there is no trade to weigh. This consciously amends the standing decision that rows dispatch above a measured ceiling: that decision assumed a rows job type the fleet does not have. MAX_INLINE_ROWS stays as the documented boundary such a job type will restore, and the previous commit's pathless-dispatch refusal backstops any routing regression. The ingestion special_note follows.
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.
Summary
Type of change
Areas touched
unify/common/_async_tool/)tests/,conftest.py,parallel_run.sh)Test plan
Behavior / migration notes
None.
Checklist
staging(notmain)feat(scope):,fix(scope):,refactor(scope):,chore(scope):, etc.)try/exceptadded defensively — only around specific, recoverable errors.agents/rules/no-temporal-comments.md).agents/rules/no-test-info-in-production-code.md)AGENTS.md/ARCHITECTURE.mdif I changed architectural conventions