[msbuild-reviewer] Add Rule B-3: catch bare-token property typos in MSBuild conditions

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M

[Amaury Levé (Evangelink)]

Summary

Adds Rule B-3 — Bare-token property reference in conditions to the msbuild-reviewer.agent rule catalog. This is a real defect class caught manually on PR #8817 that Rule B-2 does not catch (B-2 only validates that both sides of an ==/!= are quoted).

The bug class

A condition like:

<PropertyGroupCondition="'(TestingPlatformCommandLineArguments)' != ''">
<!-- ... -->
</PropertyGroup>

looks correct at a glance, but the missing \$ means MSBuild compares the literal string (TestingPlatformCommandLineArguments) to '' — which is always true — so the whole PropertyGroup is always applied regardless of whether the property is actually set. PR #8817 caught this exact mistake; without a checklist entry, the next instance will reach review the same way.

Same defect class applies to items ('@(Name)') and metadata ('%(Name)').

What changed

File	Change
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 with severity 🔴 Error and a precise reviewer trigger ("a quoted token starting with ( immediately after the opening quote"). Renumbers the remaining Category B rules (4, 5, 6) accordingly. Adds the new rule to the Severity Definitions example list.

Risk

Pure content addition to an agent's rule catalog (5 insertions, 4 deletions — the deletions are just the renumbering of rules 3→4 and 4→5). No workflow YAML changes, no .lock.yml regeneration needed. The renumber is a one-shot churn; future findings citing Rule B-3 (added here) and onward will be stable.

Validation

• git diff --stat → 1 file changed, 5 insertions(+), 4 deletions(-).
• The new rule's precise trigger ("quoted token starting with ( immediately after the opening quote") is grep-able and matches the exact pattern that surfaced in PR MSBuild Condition Fixes #8817 ('(TestingPlatformCommandLineArguments)' != '').
• No references to the renumbered rules elsewhere in the agent — the file uses Rule B-2, Rule A-3 formatting but the only existing reference to Category-B rules in the file uses the rule name, not the index.

---

🤖 Authored with GitHub Copilot CLI based on a one-month review-comment audit of microsoft/testfx (505 merged PRs sampled).

[Copilot]

Pull request overview

This PR updates the MSBuild reviewer agent rule catalog (msbuild-reviewer.agent) by adding a new Category B rule to catch a specific MSBuild condition typo pattern where a parenthesized token like '(Foo)' is mistakenly used instead of a real property reference like '$(Foo)', and renumbers the subsequent Category B rules accordingly.

Changes:

• Add Rule B-3 to flag bare-token '(Name)' usages in Condition="..." as likely property/item/metadata reference typos.
• Renumber existing Category B rules to accommodate the new rule.
• Update the Severity Definitions examples to include the new defect class.

Show a summary per file

File	Description
.github/agents/msbuild-reviewer.agent.md	Adds Rule B-3 to the rule catalog and updates Category B numbering and Severity Definitions to reflect the new rule.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Design-level note — Safe Auto-Fixes section not updated for Rule B-3

The Safe Auto-Fixes section currently allows the agent to auto-fix only B-1, B-2, and D-1 in scan mode:

- Adding Condition="'$(Prop)' == ''" to a clearly-intended default property setter (Rule B-1).
- Quoting both sides of a condition expression (Rule B-2).
- Adding Exists() guard to an obviously optional import (Rule D-1).

Correcting '(Foo)' → '$(Foo)' (Rule B-3) is a one-character, fully mechanical insertion — arguably less ambiguous than B-2 (quoting an unquoted condition requires choosing a pattern), since the intended sigil is unambiguous when a bare (Name) token is the sole content of a quoted operand. Without B-3 on this list, the agent will report the defect in the scan-mode issue but will never auto-fix it, even though the transformation is deterministic.

Suggested addition to the allow-list:

• Correcting a bare-token reference from '(Name)' to '$(Name)' in a condition (Rule B-3). For items/metadata, apply only when the surrounding context unambiguously indicates @(Name) or %(Name) was intended.

The items/metadata qualifier keeps the conservative stance consistent with the existing auto-fix constraints.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M · ◷

[Amaury Levé (Evangelink)]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Documentation-only PR (single .md agent-definition file). The defect class is real, the severity classification is correct, and the example matches the actual bug from PR #8817. 18 of 21 dimensions are N/A; the two with minor findings are noted below.

#	Dimension	Verdict
1	Algorithmic Correctness	🟡 2 NIT
21	Scope & PR Discipline	🟡 1 MODERATE

✅ 19/21 dimensions clean.

• Dim 1 — Rule B-3 trigger heuristic has two gaps: RHS comparisons not covered; items/metadata sigil-drop connection is implicit. Inline comment on line 110.
• Dim 21 — Safe Auto-Fixes section not updated. B-3 correction ('(Foo)' → '$(Foo)') is a mechanical one-character fix, equally safe as the B-2 quoting fix already on the allow-list. Separate PR comment with suggested wording.

Generated by Expert Code Review (on open) for issue #8899 · sonnet46 2.6M