[amv146]

Problem

Opening an Azure DevOps pull request reads it for the detail view, then again to discover its repository location for the diff and activity. Each redundant az repos pr show is a sequential CLI round trip.

Change

Port pingdotgg#29918 (pingdotgg#29918) by cherry-picking only feature commit dc5413d6b4da6565f1f0aca5f33c75de92935cb0, not its upstream-sync merge. Its historical head is already an ancestor of the fork and the previous integration was reverted, so merging that head would not restore the feature.

Every detail, summary, preview and checks read seeds the existing bounded location cache through a named Effect.fn helper. Diff, file and activity reads reuse only the immutable location; live pull-request details remain fresh. Preserve the existing cwd/number key, 128-entry capacity, recency and eviction rules, and the fork's newest-head-SHA regression. Extend the provider test across all four entry points and file reads, and retain/extend the eviction regression. The checks entry point came from the separately merged #3; its policy/status behavior and tests are preserved. No client, contract, workflow or lockfile changes in this PR's diff against main.

Scope and approval

The fork owner explicitly requested this separate port PR and authorized merging it into amv146/t3code main only after the latest head is ready. Review setup is complete; merging additionally requires CodeRabbit's substantive review on the latest head and handling every verified finding. The owner selected CodeRabbit only; Macroscope is suspended and not a merge dependency. This is one focused performance fix inside the Azure DevOps provider. Upstream PR state and its branch are untouched. Original author Alex Vallone and the original Copilot co-author are preserved by the cherry-pick.

This qualifies for CONTRIBUTING.md's very small, focused obvious-bug exception: a read redundantly discovers an immutable location already returned by the previous read. Reuse the established cache without changing its identity, limits, permissions or product defaults. No prior issue or discussion is required for that exception. Agent-recorded fork-owner authorization and review follow-up.

Verification

• git diff --check: passed.
• vp test run packages\source-control-azure-devops\src\server\AzureDevOpsPullRequestProvider.test.ts: blocked before running tests because local vite-plus is missing.
• vp run --filter @t3tools/source-control-azure-devops typecheck: blocked by the same missing dependency.
• vp i --frozen-lockfile: failed fetching @alchemy.run/test metadata from the corporate registry (404 Not Found). No dependency, lockfile or policy changes were made to work around it.
• vp fmt packages\source-control-azure-devops\src\server\AzureDevOpsPullRequestProvider.ts packages\source-control-azure-devops\src\server\AzureDevOpsPullRequestProvider.test.ts: blocked loading the Vite config because local vite-plus is missing.
• vp run check:changed origin/main: blocked by the same missing dependency.
• Hosted Fork PR validation at 2cd9393d9a failed before dependency installation or vp run check:changed "$BASE": actions/checkout credential cleanup runs git submodule foreach --recursive, which fails on the undeclared .repos/alchemy-effect/submodules/distilled gitlink. Failed run. The shared checkout fix is being handled separately; this port has no workflow changes. No local or hosted test pass, or new local failing-first run, is claimed.
• Merged the separately maintained checkout fix 852618b095 from origin/main normally. Hosted vp run check:changed "$BASE" at 065bffc677 passed format, lint, both knip passes and 18 tests in 2 files, but failed typecheck with three TS2722 errors in the extended provider test. Run.
• Hosted vp run check:changed "$BASE" at b45433a3bb again passed format, lint, both knip passes and all 18 tests; typecheck still found one optional summary call because its assertion had been placed in the wrong test. Run. Corrected the guard at the actual eviction-test reader in 8646a41ae98b95d94fa3681521df85ca8b3d62a9.
• Hosted vp run check:changed "$BASE" at previous head 8646a41ae98b95d94fa3681521df85ca8b3d62a9: passed format, lint, both knip passes, typecheck across 11 affected packages, and all 18 tests in 2 files. Successful run.
• CodeRabbit substantive full review completed for 8646a41ae98b95d94fa3681521df85ca8b3d62a9, with no actionable findings and no unresolved review threads. Addressed its inconclusive description check with the focused-bug exception and linked authorization record above. gh pr checks 2 --repo amv146/t3code --required reports no required checks; GitHub reports no main branch protection or active branch rules. No checks or policies were disabled or bypassed.
• The first head-pinned merge was rejected because feat(azure-devops): report pull request checks #3 had just landed with a conflicting provider test. Merged origin/main normally and retained both ports' behavior/tests. Latest head 4a6aff9bfb5f6cf65109027e12f3ca842a8873ee wires the checks reader through the same cache-seeding helper and covers it in the existing test table.
• Hosted vp run check:changed "$BASE" at latest head 4a6aff9bfb5f6cf65109027e12f3ca842a8873ee: passed format, lint, both knip passes, typecheck across 11 affected packages, and all 22 tests in 2 files. Successful integrated-head run.
• CodeRabbit substantive incremental review covers latest head 4a6aff9bfb5f6cf65109027e12f3ca842a8873ee: no actionable findings, all four pre-merge checks passed, no unresolved review threads. Required-check lookup still reports none. The queued legacy native-fingerprint job is not required; no native files changed, no checks were disabled, and no policy bypass is used.

Original feature: Claude Opus 5.5 in T3 Code. Fork adaptation: GPT-6.1 Sol (gpt-6.1-sol) through the T3 Code acpRegistry harness.

[amv146]

@coderabbitai full review

[coderabbitai]

Action performed

Full review finished.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The Azure DevOps pull-request provider now caches locations during pull-request reads. Preview, summary, checks, detail, and activity operations use the shared read path or cached location lookup. Tests cover cache seeding, repeated reads, and eviction.

Changes: Pull-request location cache

Layer / File(s)	Summary
Cache updates and lookup packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts	Pull-request reads with a non-null location update the cache. Cache misses in locationOf now use readPullRequest.
Shared reads and coverage packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts, packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.test.ts	Preview, summary, checks, and detail reads use readPullRequest. Activity reads resolve the location through locationOf. Tests cover cache seeding, repeated reads, and eviction.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: Minimal · up to 4a6af

The location-cache change is ready to merge after normal checks; no actionable regression was established.

Security architecture risk: Low · up to 4a6af

The change reuses existing Azure DevOps read operations and preserves the bounded cache. No new privilege or public contract is evident. The remaining uncertainty is whether a workspace can change repository identity while its cached location remains available to activity reads.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The directly expanded exposure is activity routing within the existing Azure DevOps provider. Distinct workspace roots have distinct cache keys. A stale entry reaching another repository would require a change in checkout identity or returned location; that production transition was not established.

Trust Boundaries and Controls

• observed — The client PR reference has no cwd field. Repository routing resolves the selected workspace before activity invokes the provider, supplying its workspace root, repository, and host. Cached locations therefore are not directly selected through a client-supplied cwd.
• observed — A cache hit removes PR-location discovery, not the downstream thread request. Threads still use the existing Azure DevOps invoke operation with checkout detection and extension credentials. The inspected source does not establish whether discovery and thread endpoints have identical server-side authorization semantics.

Resilience and Maintainability Implications

• inferred — Cache mutation is synchronous after a successful CLI read, preventing partially written entries during asynchronous interruption. Errors and null locations preserve prior cache state; misses retry discovery, and thread failures produce an empty truncated activity result. Concurrent successful reads use completion order, so correctness still depends on stable location identity. Concurrent completion and cancellation behavior were not directly tested.

Pre-merge checks: 3 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Title check		Title check was not configured for this run, so no assessment is made.	No title requirements were supplied. Provide the title requirements to evaluate this check.

Passed checks: 3 passed

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description follows the repository template and fills in the Problem, Change, Scope and approval, and Verification sections with specific detail. The Problem section is one sentence and states the…

Full details: Description check

Explanation

The description follows the repository template and fills in the Problem, Change, Scope and approval, and Verification sections with specific detail. The Problem section is one sentence and states the cause (a redundant az repos pr show read) but does not state expected behavior or a reproduction environment, which the template asks for. The Change section explains the cache seeding across detail, summary, preview, and checks reads and the preserved key, capacity, and eviction rules. The Scope and approval section cites the fork-owner authorization record and the focused-bug exception. The Verification section lists local blocked runs and hosted runs, including a passing hosted run on the latest head. The description is mostly complete, so it passes even though the Problem section is brief. Two points remain open: the Scope section relies on an agent-recorded authorization link, and the template's trailing agent attribution line is filled in.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Recent review info

Run configuration

• Source mode: Auto
• Repository: amv146/t3code
• Branch: fork/upstream-29918-location-cache
• File: .coderabbit.config.ts
• Commit: 4a6aff9
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: d9b7d9f1-b269-4784-9810-a55059e60a7b

Commits

Reviewing files that changed from the base of the PR and between 8646a41 and 4a6aff9.

Files selected for processing: 2

• packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.test.ts
• packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

[amv146]

Copilot review follow-up: this is a very small, focused fix for an obvious redundant Azure DevOps CLI read, covered by CONTRIBUTING.md's focused-bug exception. It only seeds the existing immutable-location cache from data already fetched; it preserves cache keys/limits and live detail reads, with no product-default, contract, workflow or permission changes. The fork owner explicitly requested this separate port and authorized a latest-head-ready merge in T3 Code; this comment records that authorization rather than claiming a new human review. The template comments are intentionally preserved as requested. CodeRabbit's substantive review covers 8646a41 and reports no actionable findings. Hosted validation on that exact head passed format, lint, both knip passes, typecheck of 11 affected packages and 18 tests: https://github.com/amv146/t3code/actions/runs/42517931750/job/128844928341 . The identity-transition note is not a demonstrated defect; this port deliberately preserves the existing cache identity semantics and actual reads still cross the Azure CLI authorization boundary.