ref(vsts): Remove legacy pipeline views#113336
Open
evanpurkhiser wants to merge 1 commit intomasterfrom
Open
Conversation
The VSTS (Azure DevOps) integration now uses the API-driven pipeline exclusively. Remove the old view-based pipeline steps (AccountConfigView, AccountForm, NestedPipelineView), the vsts-config.html template, and legacy test helpers. Update vsts_extension to return views directly instead of filtering the parent's views. Refs [VDY-32](https://linear.app/getsentry/issue/VDY-32/migrate-integration-setup-pipelines-to-api-driven-flows)
github-actions
bot
added
the
Scope: Backend
wedamija
approved these changes
Apr 17, 2026
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
588
to
590
| if "oauth_data" in state: | ||
| data = state["oauth_data"] | ||
| else: |
Contributor
There was a problem hiding this comment.
Bug: The fallback else branch in build_integration accesses state["identity"], but the code that populates this key has been removed, creating a latent KeyError crash risk.
Severity: LOW
Suggested Fix
The else block is dead code and should be removed. Alternatively, if it's intended as a safeguard, it should be updated to raise a more informative error, like IntegrationError("Missing oauth_data from pipeline state"), instead of crashing with a KeyError.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/integrations/vsts/integration.py#L588-L590
Potential issue: The `build_integration` function contains a fallback `else` branch that
attempts to access `state["identity"]["data"]`. This code path was intended for a legacy
pipeline flow that is being removed in this pull request. The new API-driven flow
exclusively populates `state["oauth_data"]`. If an unexpected edge case, such as
pipeline state corruption, causes `state["oauth_data"]` to be missing and the `else`
branch is executed, the application will crash with a `KeyError` because
`state["identity"]` is no longer populated by any code path.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cab95e9. Configure here.
| views = [view for view in views if not isinstance(view, AccountConfigView)] | ||
| views.append(VstsExtensionFinishedView()) | ||
| return views | ||
| return [VstsExtensionFinishedView()] |
Contributor
There was a problem hiding this comment.
Extension view pipeline missing OAuth step breaks installation
High Severity
VstsExtensionIntegrationProvider.get_pipeline_views() now returns only [VstsExtensionFinishedView()], dropping the OAuth step that was previously inherited from the parent's NestedPipelineView. The extension's view-based flow (triggered via VstsExtensionConfigurationView) jumps directly to VstsExtensionFinishedView.dispatch(), which calls pipeline.finish_pipeline() → build_integration(). Since no OAuth step ran, neither oauth_data nor identity exists in pipeline state, causing a KeyError when accessing token data. The removed test_builds_integration_with_vsts_key test was covering exactly this flow.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit cab95e9. Configure here.
| config=identity_pipeline_config, | ||
| ), | ||
| AccountConfigView(), | ||
| ] |
Contributor
There was a problem hiding this comment.
Dead legacy fallback branch after removing views
Low Severity
The TODO comment that said "Remove the legacy path once the old views are retired" was deleted, but the else branch (state["identity"]["data"]) it referred to was kept. Since the NestedPipelineView that populated state["identity"] was removed in this PR, nothing in production sets this key anymore. For the normal VSTS flow, oauth_data is always present (via API steps), making the else branch unreachable dead code.
Reviewed by Cursor Bugbot for commit cab95e9. Configure here.
Contributor
Backend Test FailuresFailures on
|
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.
The VSTS (Azure DevOps) integration now uses the API-driven pipeline
exclusively. Remove the old view-based pipeline steps (AccountConfigView,
AccountForm, NestedPipelineView), the vsts-config.html template, and
legacy test helpers. Update vsts_extension to return views directly
instead of filtering the parent's views.
Refs VDY-60