Skip to content

Commit ace9634

Browse files
Merge branch 'master' into copilot/fix-cosmos-clientbuilder-issue
2 parents a027cbf + 28ac0da commit ace9634

71 files changed

Lines changed: 6588 additions & 1136 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/agents/issue-fix-agent.agent.md

Lines changed: 216 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,11 @@ github_access:
163163
- "github-mcp-server-issue_read"
164164
- "github-mcp-server-pull_request_read"
165165
- "All github-mcp-server-* tools for Azure org"
166+
important: |
167+
Do NOT reference github-mcp-server-* tools anywhere in your workflow
168+
for Azure org repos. Always use gh CLI equivalents instead.
169+
This includes: list_issues, issue_read, pull_request_read,
170+
get_review_comments, search_code, search_issues, etc.
166171
167172
recommended_approach:
168173
primary: "Use gh CLI for all GitHub operations"
@@ -685,6 +690,38 @@ troubleshooting:
685690
fix: "First MCP call opens browser - complete Microsoft login"
686691
```
687692
693+
### 0.8 Whitespace & Line Endings
694+
695+
```yaml
696+
line_endings:
697+
git_config:
698+
# Repository uses core.autocrlf=input (LF in repo, no conversion on checkout)
699+
# Verify with: git config core.autocrlf
700+
expected: "input"
701+
702+
common_pitfall: |
703+
When editing a file, some editors normalize all line endings to LF or CRLF.
704+
If the original file has mixed line endings (some CRLF, some LF), your edit
705+
may produce a diff with spurious whitespace-only changes that reviewers will flag.
706+
707+
prevention:
708+
before_editing:
709+
- "Check original line endings: git show master:{file} | python -c \"import sys; d=sys.stdin.buffer.read(); print(f'CRLF:{d.count(chr(13).encode()+chr(10).encode())} LF:{d.count(chr(10).encode())}')\""
710+
711+
after_editing:
712+
- "Verify no whitespace-only changes: git diff master --ignore-all-space -- {file}"
713+
- "If whitespace diff appears, restore from master and re-apply only intentional changes"
714+
715+
restore_pattern: |
716+
# If you accidentally introduced whitespace changes:
717+
git checkout master -- {file} # Restore original
718+
# Then re-apply only your intentional additions using edit tool
719+
720+
diff_verification:
721+
always_run: "git diff master --ignore-cr-at-eol -- {file}"
722+
if_spurious_changes: "git checkout master -- {file} and re-apply edits"
723+
```
724+
688725
---
689726
690727
## 1. Executive Summary
@@ -796,19 +833,24 @@ mandatory_checkpoints:
796833
- "Search for existing PRs: `gh pr list --search 'ISSUE_NUMBER'`"
797834
- "Search for existing branches: `git branch -a | grep ISSUE_NUMBER`"
798835
- "If PR exists, use existing branch instead of creating new one"
836+
- "If existing PR is stale (no activity >30 days), comment on it and create new branch"
799837
- "Document search results as evidence"
800838

801839
before_creating_pr:
802840
- "Local build MUST succeed with captured output"
803841
- "Local tests MUST pass with captured output"
804842
- "Root cause MUST be documented with code path"
805843
- "Fix MUST be verified with before/after evidence"
844+
- "If before/after evidence is not possible (environment-specific issue), document why and provide unit test evidence instead"
806845
- "PR MUST be created as DRAFT: `gh pr create --draft`"
846+
- "PR title MUST follow format: `[Internal] Category: (Adds|Fixes|Refactors|Removes) Description` (enforced by PR Lint bot)"
847+
- "Verify no spurious whitespace changes: `git diff master --ignore-all-space`"
807848

808849
before_marking_pr_ready:
809-
- "ALL CI gates MUST show COMPLETED status"
850+
- "ALL CI gates MUST show green (passed) status — not just completed"
810851
- "PR description MUST follow full template"
811852
- "Issue comment MUST be posted with investigation summary"
853+
- "All reviewer comments MUST be addressed (see Section 7.4.1)"
812854
- "Convert from draft: `gh pr ready <PR_NUMBER>`"
813855

814856
before_claiming_issue_resolved:
@@ -2437,6 +2479,98 @@ Based on area labels, assign to: @{reviewer}
24372479

24382480
---
24392481

2482+
### 4.9 Compiled Dependencies (NuGet Transport Layer)
2483+
2484+
```yaml
2485+
compiled_dependencies:
2486+
context: |
2487+
The SDK's Direct mode transport layer (Connection.cs, StoreClientFactory.cs, etc.)
2488+
is compiled into a NuGet package (Microsoft.Azure.Cosmos.Direct). Source code for
2489+
these files is NOT in the repository — they exist only in a separate internal repo
2490+
and are consumed as binary references.
2491+
2492+
identifying_compiled_code:
2493+
signs:
2494+
- "File referenced in code but not found with `find` or `glob`"
2495+
- "Class/method visible via IntelliSense but no .cs source file in repo"
2496+
- "grep finds usage sites but not the definition"
2497+
confirm: "Check .csproj for PackageReference to Microsoft.Azure.Cosmos.Direct"
2498+
2499+
working_with_compiled_code:
2500+
cannot_do:
2501+
- "Modify internal methods (e.g., Connection.ResolveHostAsync)"
2502+
- "Change constructor signatures"
2503+
- "Access private/internal members from the main SDK assembly"
2504+
can_do:
2505+
- "Use public/internal API surface exposed by the package"
2506+
- "Use injection points (e.g., dnsResolutionFunction parameter)"
2507+
- "Wrap compiled functionality with custom delegates/functions"
2508+
2509+
strategy_when_plan_references_compiled_code: |
2510+
If your implementation plan references modifying compiled code:
2511+
1. STOP — verify the file exists in the repo first
2512+
2. If NOT in repo, find the public injection point (constructor parameter, interface, etc.)
2513+
3. Implement your logic in the main SDK assembly and inject via the public API
2514+
4. Update your plan to document the deviation
2515+
2516+
example: |
2517+
Plan said: "Modify Connection.ResolveHostAsync() to add trailing dot"
2518+
Reality: Connection.cs is in compiled NuGet, not in repo
2519+
Fix: Use StoreClientFactory's dnsResolutionFunction parameter to inject
2520+
a custom resolver that wraps Dns.GetHostAddressesAsync() with dot-suffix logic
2521+
```
2522+
2523+
### 4.10 Environment-Specific Issues
2524+
2525+
```yaml
2526+
environment_specific_issues:
2527+
context: |
2528+
Some issues only manifest in specific environments (Kubernetes, Linux, specific
2529+
cloud regions, multi-region setups). These cannot be reproduced on a local Windows
2530+
development machine with the Cosmos DB emulator.
2531+
2532+
identification:
2533+
keywords:
2534+
- "Kubernetes", "k8s", "ndots", "pod", "container"
2535+
- "Linux", "Alpine", "Docker"
2536+
- "multi-region", "failover", "cross-region"
2537+
- "proxy", "SNAT", "NAT gateway"
2538+
- "connection pooling", "socket exhaustion"
2539+
2540+
when_reproduction_impossible:
2541+
acceptable_evidence:
2542+
- "Unit tests that verify the fix logic in isolation"
2543+
- "Integration tests that verify the injection point works"
2544+
- "Documented explanation of why the environment cannot be replicated"
2545+
- "Link to external documentation confirming the behavior (e.g., Kubernetes DNS docs)"
2546+
2547+
before_after_alternatives:
2548+
- "Show conceptual before/after (DNS query flow, connection sequence, etc.)"
2549+
- "Show unit test before/after (input → output)"
2550+
- "Reference customer-reported metrics if available"
2551+
2552+
pr_description: |
2553+
In the "Generated Output (Before/After)" section, clearly state:
2554+
"Note: This behavior only manifests in {environment}. Before/after evidence
2555+
is conceptual — unit tests verify the fix logic."
2556+
2557+
common_patterns:
2558+
kubernetes_dns:
2559+
issue: "ndots:5 causes search-domain expansion latency"
2560+
cannot_reproduce: "Local Windows DNS resolver doesn't use ndots"
2561+
evidence: "Unit tests for hostname transformation + Kubernetes docs reference"
2562+
linux_specific:
2563+
issue: "Different socket/TLS behavior on Linux"
2564+
cannot_reproduce: "Windows emulator only"
2565+
evidence: "Conditional compilation tests + runtime platform detection"
2566+
multi_region:
2567+
issue: "Failover, consistency, replication lag"
2568+
cannot_reproduce: "Emulator is single-region"
2569+
evidence: "Mock-based tests + CI multi-region pipeline (requires secrets)"
2570+
```
2571+
2572+
---
2573+
24402574
## 5. Investigation Issue Creation
24412575
24422576
> **Note:** Section 5 covers creating investigation issues for complex bugs that require detailed tracking. The investigation issue template is detailed in Section 4's "Investigation Issue Template" subsection.
@@ -2569,7 +2703,41 @@ do_not_create_pr_when:
25692703
- Security-sensitive fix (escalate instead)
25702704
```
25712705
2572-
### 7.3 PR Template
2706+
### 7.3 PR Title Format
2707+
2708+
**The repository has a PR Lint bot that enforces title format. PRs with incorrect titles will be flagged.**
2709+
2710+
```yaml
2711+
pr_title:
2712+
format: "[Internal] Category: (Adds|Fixes|Refactors|Removes) Description"
2713+
2714+
rules:
2715+
- "[Internal] prefix is for PRs with no customer impact (internal refactoring, CI changes, test additions)"
2716+
- "Omit [Internal] for customer-facing changes (bug fixes, new features, behavior changes)"
2717+
- "Category matches the SDK area: Query, LINQ, Batch, DirectMode, ChangeFeed, SDK, Diagnostics, etc."
2718+
- "Verb must be one of: Adds, Fixes, Refactors, Removes"
2719+
2720+
decision_tree:
2721+
is_customer_facing:
2722+
yes: "Category: Verb Description"
2723+
no: "[Internal] Category: Verb Description"
2724+
verb_selection:
2725+
new_feature: "Adds"
2726+
bug_fix: "Fixes"
2727+
code_restructure: "Refactors"
2728+
deprecated_removal: "Removes"
2729+
2730+
examples:
2731+
- "DirectMode: Adds DNS dot-suffix to avoid Kubernetes ndots latency"
2732+
- "LINQ: Fixes Dictionary.Any() to generate correct SQL with OBJECTTOARRAY"
2733+
- "[Internal] Tests: Adds unit tests for DnsDotSuffixHelper"
2734+
- "SDK: Adds support for hierarchical partition keys"
2735+
- "Query: Refactors SQL generation for better readability"
2736+
2737+
fix_if_rejected: "gh pr edit {pr_number} --title 'Category: Verb Description'"
2738+
```
2739+
2740+
### 7.3.1 PR Description Template
25732741
25742742
**PR must include full investigation details, not just a summary.**
25752743
@@ -2750,6 +2918,52 @@ branch_to_pr_url:
27502918
| Serialization | @serialization-owners | @sdk-team |
27512919
| Encryption | @encryption-owners | @security-team |
27522920
2921+
### 7.4.1 Handling Reviewer Feedback
2922+
2923+
```yaml
2924+
reviewer_feedback_workflow:
2925+
monitoring:
2926+
when: "After PR is created and after each push"
2927+
how: "gh api repos/{owner}/{repo}/pulls/{pr}/comments --jq '.[] | {id, path, line, body, user: .user.login}'"
2928+
frequency: "Check after each push; respond before pushing more changes"
2929+
2930+
addressing_comments:
2931+
steps:
2932+
1_acknowledge: |
2933+
Read all inline review comments before making changes.
2934+
Group related comments to avoid multiple fix-then-break cycles.
2935+
2_implement: |
2936+
Make all requested changes in a single commit.
2937+
Commit message: "Address PR review: <summary of changes>"
2938+
3_reply: |
2939+
Reply to EACH review comment with what was done:
2940+
gh api repos/{owner}/{repo}/pulls/{pr}/comments/{comment_id}/replies \
2941+
-f body="Done. <brief description of change> in <commit_sha_short>."
2942+
4_verify: |
2943+
Rebuild and rerun tests after addressing feedback.
2944+
Push only after local verification passes.
2945+
2946+
common_review_patterns:
2947+
naming_feedback:
2948+
action: "Rename as requested, update all references, reply with new name"
2949+
whitespace_noise:
2950+
action: "Restore original file from master, re-apply only intentional changes"
2951+
command: "git checkout master -- {file} && re-apply edits"
2952+
missing_tests:
2953+
action: "Add requested tests, show pass evidence in reply"
2954+
style_issues:
2955+
action: "Follow existing code patterns in the file being modified"
2956+
2957+
disagreements:
2958+
approach: "Do NOT argue. Implement reviewer's suggestion unless it introduces a bug."
2959+
if_reviewer_wrong: "Explain politely with evidence (code path, test output). Let reviewer decide."
2960+
escalation: "If deadlocked after 2 rounds, ask reviewer to suggest specific alternative."
2961+
2962+
re_review:
2963+
after_pushing_fixes: "GitHub automatically requests re-review on new pushes to draft PRs"
2964+
manual: "gh pr edit {pr} --add-reviewer {reviewer} (only if needed)"
2965+
```
2966+
27532967
### 7.5 Remote CI Validation (Azure Pipelines Gates)
27542968
27552969
**Critical: Local tests are not sufficient. All fixes must pass the full Azure Pipelines CI gates.**

0 commit comments

Comments
 (0)