Skip to content

fix(server): support Azure DevOps pull request responses - #8364

Closed
Kieren-Foenander wants to merge 7 commits into
pingdotgg:mainfrom
Kieren-Foenander:feature/fix/azure-devops-pull-requests
Closed

fix(server): support Azure DevOps pull request responses#8364
Kieren-Foenander wants to merge 7 commits into
pingdotgg:mainfrom
Kieren-Foenander:feature/fix/azure-devops-pull-requests

Conversation

@Kieren-Foenander

@Kieren-Foenander Kieren-Foenander commented Aug 27, 2026

Copy link
Copy Markdown

What Changed

  • Read Azure DevOps pull request threads through az devops invoke, reusing the Azure CLI extension authentication.
  • Accept nullable Azure DevOps response fields and normalize thread route data.
  • Fall back to fetching the source branch for same-repository Azure DevOps pull requests, which do not publish a pull-request head ref.
  • Add focused coverage for response decoding, CLI invocation, pull request listing, and branch fetching.

Related issue

Fixes #8078

Why

Azure DevOps responses can contain nullable fields, and same-repository pull requests do not expose the synthetic head refs used by other providers. Those differences caused pull request loading, activity reads, or checkout to fail. This keeps the provider-specific behavior at the Azure DevOps boundary while preserving the existing orchestration flow.

Verification

  • vp test run apps/server/src/git/GitManager.test.ts apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts apps/server/src/pullRequest/azureDevOpsPullRequestJson.test.ts apps/server/src/sourceControl/AzureDevOpsCli.test.ts apps/server/src/sourceControl/AzureDevOpsSourceControlProvider.test.ts (162 tests passed)
  • vp run --filter t3 typecheck
  • Targeted lint and formatting checks for all changed files

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • No UI changes

Created with GPT-5.6 Codex in the T3 Code harness.


Note

Medium Risk
Changes pull-request head materialization and Azure fork identity handling, which can affect worktree checkout for cross-repo PRs, but ambiguous forks fail explicitly and same-repo Azure behavior is a targeted fallback.

Overview
Hardens Azure DevOps pull request handling so nullable API payloads, fork metadata, and missing PR head refs no longer break loading, threads, or worktree checkout.

Azure normalization relaxes the PR JSON schema for null optional fields, reads forkSource to set isCrossRepository and head repo identity (project/repo), and surfaces those fields through the Azure DevOps source-control provider instead of always treating PRs as same-repo.

Checkout / materialization in GitManager limits the “fetch by PR ref only” path to same-repository PRs; when that fetch fails (e.g. Azure same-repo PRs with no pull head ref), it falls back to fetching the remote source branch by name. Cross-repo PRs with no resolvable head repository now fail closed with GitPullRequestMaterializationError instead of guessing from a same-named local branch.

CLI tweaks: PR thread REST calls include the Azure DevOps resource GUID; getRepositoryCloneUrls accepts project/repo and passes --project to az repos show.

Coverage adds scenarios for null fields, Azure forks, ambiguous fork identity, no-pull-ref worktrees, and authenticated thread invocation.

Reviewed by Cursor Bugbot for commit 8ff6067. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix Azure DevOps pull request parsing and GitManager worktree materialization

  • Azure pull-request JSON decoder now accepts null optional fields and retains fork-source metadata, so fork PRs are marked cross-repository with head repository identity and owner in azureDevOpsPullRequests.ts
  • AzureDevOpsSourceControlProvider.toChangeRequest copies cross-repository and head-repository fields into provider-neutral change requests instead of hard-coding same-repository
  • AzureDevOpsPullRequestCli.listThreads adds the Azure DevOps resource identifier required for authenticated REST access
  • AzureDevOpsCli.getRepositoryCloneUrls splits project-qualified repository specs into separate repository and project arguments
  • GitManager.materializePullRequestHeadBranchBase falls back to fetching the head branch from the primary remote for same-repository PRs; cross-repository PRs without a resolvable head repository now fail closed with GitPullRequestMaterializationError
  • Risk: cross-repository Azure PRs with incomplete fork metadata will fail materialization rather than attempt an ambiguous checkout — check normalizeAzureDevOpsPullRequestRecord in azureDevOpsPullRequests.ts and the precondition in materializePullRequestHeadBranch in GitManager.ts

Macroscope summarized 8ff6067.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d48b73c4-b35c-4998-b444-4b9e155eafc6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 27, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions review: one change-discipline finding — documentation that recorded why the threads route is derived from Azure's response was dropped while threadsUrl was refactored into the threads route. Service definitions, layers, namespace imports, error modelling, and the added tests otherwise follow the conventions.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts Outdated
Comment thread apps/server/src/pullRequest/azureDevOpsPullRequestJson.ts Outdated
@Kieren-Foenander
Kieren-Foenander force-pushed the feature/fix/azure-devops-pull-requests branch from 01913b5 to 8b1ecde Compare August 27, 2026 07:34
Comment thread apps/server/src/git/GitManager.ts
@macroscopeapp

macroscopeapp Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a focused Azure DevOps compatibility fix, but it changes production pull-request materialization behavior and adds new line-level static-analysis diagnostic suppressions in test code. Those suppressions require human review before merging.

You can add or adjust custom eligibility rules. Learn more.

@Kieren-Foenander
Kieren-Foenander force-pushed the feature/fix/azure-devops-pull-requests branch from 8b1ecde to 2f9e77c Compare August 27, 2026 07:53
@Kieren-Foenander

Copy link
Copy Markdown
Author

Addressed the approvability concern in 2f9e77c3. Azure DevOps documents forkSource as the discriminator and source repository for fork PRs. The decoder now preserves that metadata, the source-control provider marks those PRs cross-repository, and GitManager uses its existing fork-remote checkout path. A fork with incomplete identity now fails closed rather than falling back to a same-named primary-remote branch. Regression coverage verifies both decoding and provider propagation; 161 focused tests pass.

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Aug 27, 2026
Comment thread apps/server/src/sourceControl/azureDevOpsPullRequests.ts
- Read pull request threads through the Azure DevOps CLI
- Handle nullable Azure fields and same-repository PR refs
@Kieren-Foenander
Kieren-Foenander force-pushed the feature/fix/azure-devops-pull-requests branch from 2f9e77c to f40dbd3 Compare August 27, 2026 08:18
Comment thread apps/server/src/sourceControl/azureDevOpsPullRequests.ts Outdated
@Kieren-Foenander

Copy link
Copy Markdown
Author

before vs after testing.

image image

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5a14e10. Configure here.

Comment thread apps/server/src/git/GitManager.ts
@juliusmarminge

Copy link
Copy Markdown
Member

The command-construction defect in #8078 remains reproducible on current main. An isolated production AzureDevOpsPullRequestCli.listThreads fixture records az rest without a token resource. The tested implementation at dd7bc147 is unchanged through September 4 main 0dd5c64b.

Needs human review. This PR also changes nullable response handling, fork identity, and branch materialization, which that reproduction does not validate. The author's live screenshots are useful prior evidence, but I have not rerun those authenticated flows. Coordinate the authentication choice with #9311, then verify same-repository and fork checkout paths on the selected current head.

GPT 6 Astra via Codex in T3 Code.

@Kieren-Foenander

Copy link
Copy Markdown
Author

Updated the branch to current main in d04a3ca and addressed the latest review in 8ff6067. Authentication now follows #9311: Azure thread reads retain the existing REST URL and pass the documented Azure DevOps resource ID, with the complete command covered by a regression test. Incomplete fork identity now fails before repository auto-detection or PR-ref fallback. Verification: 181 focused tests pass, targeted lint passes, and the server typecheck passes. I did not run a live authenticated Azure DevOps account check.

@Kieren-Foenander

Copy link
Copy Markdown
Author

Closing in favor of #9311, which is the focused fix for #8078: it adds the Azure DevOps resource ID to the existing REST request with a narrow regression test. Current main still lacks that fix. The remaining nullable-response, fork-identity, and checkout changes here are broader than the reported comments bug and should be proposed separately if we have concrete reproduction or live validation for them. The branch and commits remain available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Azure DevOps PR comments show “most recent 0” because thread request lacks DevOps resource authentication

2 participants