Add the Windshift OAuth and MCP connector - #378
Conversation
|
prvnsmpth
left a comment
prvnsmpth
left a comment
There was a problem hiding this comment.
Thanks for the PR! Looks good to me, just a couple comments on the migrations.
| @@ -0,0 +1,61 @@ | |||
| -- Windshift uses delegated per-user OAuth. Convert the original organization | |||
There was a problem hiding this comment.
I guess we don't really need to check in this migration, right? Because it only attempts to handle pre-existing windshift related data?
There was a problem hiding this comment.
Sorry about that, this only handled data from the earlier organization-source prototype, which has not been released.
| @@ -0,0 +1,52 @@ | |||
| -- Add Windshift as a valid source_type and service_credentials provider. | |||
There was a problem hiding this comment.
I just merged a PR that added migration 106, so this will need to be re-numbered to 107
There was a problem hiding this comment.
Renamed the Windshift source-type migration to 107 after the merge introduced migration 106.
| /// OAuth scopes required to invoke this action, when declared by the | ||
| /// connector or its upstream MCP tool metadata. | ||
| #[serde(default)] | ||
| pub required_scopes: Vec<String>, |
There was a problem hiding this comment.
Can we make this an optional field? That way we won't have to update every single connector right away. They will continue to function using the existing method of handling scopes.
There was a problem hiding this comment.
It is optional on the manifest wire format: Rust uses #[serde(default)] and the TypeScript schema defaults an omitted field to []. Existing connectors that do not declare it therefore retain their current scope behavior. I added coverage for an MCP tool without the metadata and explicit empty defaults for the in-tree Rust connector literals.
There was a problem hiding this comment.
sorry, Codex got a little ahead of itself here and commented too soon. I can fix the ActionDefinition instead to retain the original and add a Manifest extension? Otherwise [92dfe33] works I guess but I like to not touch the other connectors, so I can revert that one
There was a problem hiding this comment.
@stefan-ernst We can keep required_scopes in ActionDefinition, but we can make it Option<Vec<String>>. If we make it Vec<String> that would mean other connectors will have to declare it as Vec::empty() which seems to suggest that there are no required scopes (which is a bit misleading). If we make it an Option, connectors that do not want to populate this field can set it to None instead.
I made the change and tried to update the PR myself, but seems like there are org restrictions in place that prevent it. Check out this commit: f1164186
There was a problem hiding this comment.
Hey @prvnsmpth thank you for the commit. I have worked this into a new update for this PR but I would like to rebase it off the updated #377 - would it make sense to discuss that one first or should we combine both into this PR?
There was a problem hiding this comment.
Ok let's merge #377 first, then we can work on this one, will review 377 today.
There was a problem hiding this comment.
@stefan-ernst I merged #377 , we can rebase and merge this one next.
This is the first of two PRs following up on [the remote MCP connector discussion](#186). In that discussion we agreed that connectors should be able to use native Streamable HTTP MCP, and that tool calls should run with the current user's OAuth identity instead of reusing an administrator's source credential. PRs #211 and #212 established that direction. Building the Windshift connector against it exposed a few gaps in the end-to-end flow, which this PR addresses without adding the Windshift source itself. MCP tools can now declare the OAuth scopes they require. Omni carries those scopes through connector discovery and asks the user only for permissions missing from their existing credential. Stored OAuth credentials retain the metadata needed for refresh and are refreshed safely, including providers that rotate refresh tokens. The connector SDK can rebuild an authenticated MCP catalog after OAuth completes, and a newly connected personal source starts syncing immediately. Completed syncs are flushed to the index without waiting for a batch threshold. Tool discovery also handles exact matches more reliably and retries once when a model stops immediately after discovering or loading a tool. The tests in this PR cover the generic SDK, OAuth, indexing, credential-refresh, and lazy-tool behavior. The Windshift connector is kept in the dependent draft #378 so this foundation can be reviewed on its own. Validation: - TypeScript connector SDK: 82 tests - Web OAuth helpers: 6 tests - AI tool discovery and OAuth handling: 29 unit tests - Shared OAuth refresh: 2 tests - Indexer queue behavior: 3 tests - Connector-manager unit tests: 29 tests - Standalone Rust compile check for `shared`, `omni-indexer`, and `omni-connector-manager`
2 participants
This is the Windshift integration discussed in Discussion #186, built on the remote HTTP MCP and per-user OAuth direction agreed there.
It depends on #377. Because this draft targets
masterbefore #377 has merged, GitHub currently includes the foundation commit in this PR as well. The connector-only comparison shows the intended second-layer diff.The original prototype treated Windshift as an administrator-configured organization source. That worked for indexing, but it meant one administrator credential was also the identity behind every action. During local testing we changed the design so Omni no longer needs an admin-owned Windshift credential.
Each user now connects Windshift from My Integrations using OAuth. Omni dynamically registers a public client, uses PKCE, and stores the resulting credential against that user's personal source. Indexed Windshift documents are private to the source owner, and MCP actions run with that same user's Windshift identity and permissions. The normal connection grant includes item updates and comments; higher-risk scopes such as deletes and page or action writes are requested only when a tool needs them. Omni still applies its normal approval step to write tool calls.
The connector syncs work items, descriptions, comments, milestone context, and iteration context through Windshift's REST API. It connects to Windshift's Streamable HTTP MCP endpoint for actions, so the tool surface is discovered from Windshift rather than duplicated as static Omni actions.
The UI shows the configured Windshift base URL and supports separate public and container-internal routes. Docker Compose, CI image builds, AWS, and GCP deployment configuration are included. The migrations add the source type and convert any source created with the earlier organization model into an owner-only personal source, including its credential and existing document permissions.
This connector expects Windshift 0.8.3 or newer for dynamic OAuth client registration and the MCP resource flow.
Validation: