Note to reviewers - This and the code in schema.ts was added to fix the Workflow YAML editor giving warnings when stringing together the ouputs of one SPO step with other steps.

Cards on the table, I kind of let Cursor take the reigns on these particular schema code diffs. They don't seem to have any adverse side effects but if others think they'd be better served as a separate PR I can remove them.

Should we try to use the /content URL first, before we return a link to a download URL?

This downloadUrl path feels misleading to me. I'd expect that to be the input to a downloadDriveItem tool, not the output.

Asking some clarifying questions 😄

Should we try to use the /content URL first, before we return a link to a download URL?

Are you suggesting we should have a more automated behavior, where we attempt /content and simply return downloadUrl to the user if those fail (most likely bc its a PDF or .docx or something). Not entirely opposed to this if so, just want to make sure I understand. I have a lightly held opinion that its not amazing if actions return two different types of data given a single input set - either doc contents or a URL - but again, it's lightly held.

I'd expect that to be the input to a downloadDriveItem tool, not the output.

The way I envisioned this working was it'd accept a driveId and itemId as input, as they are outputs of other tools, then attempt get the content in a format the LLM could work with, and then give the user the downloadUrl if it can't...which means, In its current form, the LLM would call this tool again with a different input set. I'm not sure how ideal that behavior is, either.

A big simplification would be to just have getDriveItems return the downloadUrl as part of its payload and then have this action only accept downloadUrls as input, but I'd have to test how well this works.

Hopefully I've understood what your concerns are, if not please course-correct me 🫡

Are you suggesting we should have a more automated behavior, where we attempt /content and simply return downloadUrl to the user if those fail

I haven't done any googling or investigation here. So take this with a grain of salt. But my concern is that we might have some responses where there's a content endpoint that can:

1. give us the actual text contents

and there's a dowloadURL where we could:

1. download a big file
2. ship that big file somewhere else for processing
3. get the actual text contents

If we can jump straight to the text, I want to make sure we do, to avoid unnecessary bandwidth, compute, and latency.

A big simplification would be to just have getDriveItems return the downloadUrl as part of its payload and then have this action only accept downloadUrls as input, but I'd have to test how well this works.

I think that could be a good piece of metadata to include. The flow I'd expect is:

1. getMany (mostly metadata)
2. LLM introspection (do any of these look promising?)
3. fetchOneorSeveral (full contents, using metadata from 1 to limit the number of results)

If downloadUrl isn't included in 1, and another pass has to be done to get that metadata, this feels inefficient.

Ok, I think I got it:

1. We should make sure to return downloadUrls as part of our getMany-style actions for drive items
2. I'm going to create a new action (downloadItemFromURL or something) that can acquire the file via a downloadUrl, which we can hopefully use as an action for shipping more complicated formats elsewhere for processing
3. I'm going to remove the downloadUrl code path from the current downloadDriveItem action. This existing action should just be our way of getting straight to text content. I'll also remove the base64 encoding format too.

Net result will be clearer definitions:

• An action (downloadDriveItem) that can directly get text content
• Another action (downloadItemFromURL) can download a file via URL to potentially farm out for further processing

🤞 Hopefully I've understood the concerns and provided a good way forward!

👍 this sounds like a reasonable path forward. Thanks!

Note to reviewers - apparently region is required for Search after all :(

It's the only place where it's required, so I've made it an optional input with NAM as the default, so an LLM can always pass it if necessary. This way, we don't have to ask for it at connector set-up time.

That being said, I don't feel terribly strongly about this, so if we want to revert to asking for a region at set-up time I can do so.

Nice, I like this solution.

We can have search region be an optional field, something like:

schema: z.object({
    region: z
      .enum(['NAM', 'EUR', 'APC', 'LAM', 'MEA'])
      .optional()
      .default('NAM')
      .describe(
        'Search region (NAM=North America, EUR=Europe, APC=Asia Pacific, LAM=Latin America, MEA=Middle East/Africa)'
      )
      .meta({
        label: 'Search region',
      }),
  }),

Feel free to leave this for a follow-up, and only if we're interested.

Wait, sanity check: we're doing SPO connector that does not run on behalf of the user, is it correct?

That's correct I believe, it does not run on the behalf of a user

@artem-shelkovnikov , the "on behalf of the user" bit is blocked by #246655.

Once we merge that, we can simply change this line to

type: 'oauth_authorization_code_grant'

instead of

type: 'oauth_client_credentials'

and everything should "just work".

But that PR is probably going to be blocked for a bit. Hence the temporary alternative auth mechanism.

I think it'll be a bit different in details - for example there's an endpoint in the PR that uses region param because it's needed by application permission requests.

We'd need to update some endpoints here and there + update the instructions for setting up the delegated permissions instead of application permissions