Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .github/agents/expert-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,15 @@ Hot paths: `Evaluator.cs`, `Expander.cs`, file I/O operations.
2. Name test methods to describe scenario and outcome (e.g., `PropertyOverride_LastWriteWins_InImportedProject`).
3. Tests must be deterministic — no dependency on file system ordering, timing, or uncontrolled environment.
4. Test both positive and negative paths, including edge cases.
5. Verify test assertions actually validate the claimed behavior — weak assertions (e.g., "not empty", "no crash") can pass even with incorrect output.

**CHECK — Flag if:**
- [ ] New behavior has no test coverage
- [ ] A bug fix has no regression test
- [ ] Test method names are opaque (e.g., `Test1`, `TestBug`)
- [ ] Tests depend on implicit environment state
- [ ] Only the happy path is tested
- [ ] Test assertions are too weak to catch incorrect behavior (would pass with wrong output)

---

Expand Down Expand Up @@ -227,12 +229,18 @@ See `../../documentation/wiki/Microsoft.Build.Framework.md`.
2. Complex features require a written spec — see `../../documentation/specs/`.
3. Make incremental, reviewable commits. Large monolithic PRs are rejected.
4. Follow established design patterns. Don't introduce new patterns without discussion.
5. For bug fix PRs, read the original issue and feature PR discussions to understand the design intent. Verify the fix aligns with it.
6. When code works around an API limitation (try/catch chains, TOCTOU patterns, fallback sequences), check whether a better API exists in already-referenced packages that would eliminate the workaround.
7. When a pattern is borrowed from another codebase or context, verify its assumptions still hold in the new context.

**CHECK — Flag if:**
- [ ] Large feature PR with no linked spec
- [ ] New architectural pattern introduced without discussion
- [ ] Single PR mixes multiple unrelated concerns
- [ ] Design trade-offs not articulated
- [ ] Fix contradicts design intent established in original feature discussions
- [ ] Workaround for an API limitation when a better API is available in existing dependencies
- [ ] Pattern borrowed from a different context without validating its assumptions apply here

---

Expand Down Expand Up @@ -460,13 +468,17 @@ See `../../documentation/High-level-overview.md` and `../../documentation/Built-
3. Validate fixes against original repro steps.
4. Validate inputs early — fail fast with clear errors.
5. Imagine exotic scenarios that could break assumptions. Like a red teamer trying to break MSBuild with weird project files or build environments.
6. When a fix handles N=2 participants (e.g., two concurrent writers), verify it also works for N=3+. Fixes that close one race window often leave a wider one.
7. Verify the fix addresses the root cause, not just a symptom. Patching over a structural issue (e.g., adding retries around a TOCTOU) may need revisiting if a cleaner solution exists.

**CHECK — Flag if:**
- [ ] New code path doesn't handle null/empty inputs
- [ ] Bug fix doesn't include original repro as test
- [ ] Boundary conditions not considered (off-by-one, max-length, empty)
- [ ] Input validation missing at public API boundaries
- [ ] Code relies on assumptions that exotic inputs could break
- [ ] Fix only handles the 2-participant case but fails with more concurrent actors
- [ ] Fix patches a symptom when the root cause could be addressed

---

Expand Down Expand Up @@ -550,6 +562,8 @@ Use this to prioritize dimensions based on changed files.

1. Map changed files to the [Folder Hotspot Mapping](#folder-hotspot-mapping).

1b. **Historical context** (for bug fix and follow-up PRs): Read the linked issue and the original feature PR discussions. Identify design intent, constraints, and reviewer-established principles. Feed this context to every dimension agent so they can evaluate whether the fix aligns with the original design, not just whether the code compiles.
Comment thread
JanProvaznik marked this conversation as resolved.

2. Launch **one sub-agent per dimension** (`task` tool, `agent_type: "general-purpose"`, `model: "claude-opus-4.6"`). Each agent evaluates exactly one dimension against the full PR diff. Run in **parallel batches of 6** (4 batches for 24 dimensions).

Each sub-agent receives: the PR diff, PR description, the single dimension's rules and checklist, and the folder context.
Expand Down
1 change: 1 addition & 0 deletions .github/instructions/tasks.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Built-in tasks ship with MSBuild and cannot be independently versioned.

* Use `FileUtilities` helpers — do not roll custom path manipulation.
* Support UNC paths, long paths (> 260 chars), and cross-platform separators.
* The `Microsoft.IO.Redist` package is referenced and backports .NET Core file APIs to .NET Framework. Check `Microsoft.IO.File`, `Microsoft.IO.Path`, `Microsoft.IO.Directory` for better overloads before writing workarounds (e.g., `Microsoft.IO.File.Move(src, dst, overwrite: true)` eliminates TOCTOU patterns around `File.Move` + `File.Replace`).

## Multithreaded Task Migration

Expand Down
Loading