Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Make 'never APPROVE' rule prominent in expert-reviewer prompt - #8221

Merged
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve
May 14, 2026
Merged

Make 'never APPROVE' rule prominent in expert-reviewer prompt#8221
Amaury Levé (Evangelink) merged 1 commit into
mainfrom
fix/expert-reviewer-no-approve

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Symptom

When the Expert Code Review (on open) workflow ran on PR #8220, the agent successfully reviewed the change but tried to submit it with event: APPROVE. The safe-outputs filter (allowed-events: [COMMENT, REQUEST_CHANGES]) correctly rejected it, and the whole safe_outputs job failed:

https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833

##[warning]Review event 'APPROVE' is not allowed. Allowed events: COMMENT, REQUEST_CHANGES
##[error]✗ Message 2 (submit_pull_request_review) failed: Review event 'APPROVE' is not allowed by safe-outputs configuration. Allowed events: COMMENT, REQUEST_CHANGES
##[error]1 safe output(s) failed

The verdict summary table — and all inline review comments bundled into that submit_pull_request_review call — were dropped on the floor as a side effect.

Root cause

submit-pull-request-review.allowed-events: [COMMENT, REQUEST_CHANGES] is intentional: this agent must not be able to count as a PR approval. The agent's prompt does say Never use APPROVE, but the rule sits ~630 lines into .github/agents/expert-reviewer.agent.md, well after 21 dimension definitions and a four-wave workflow. The model didn't carry it through to the final tool call.

Fix

Reinforce the constraint in three places to make it impossible to miss:

  1. .github/agents/expert-reviewer.agent.md — Add an Absolute Rules (read first, must never be violated) section right under the title. Rule Initial commit! 🎉 #1 explicitly forbids event: ""APPROVE"" and spells out the consequence (entire review, including bundled inline comments, is dropped).
  2. .github/agents/expert-reviewer.agent.md — Strengthen the existing Wave 4 step-6 reminder with a stop sign and a back-reference to the Absolute Rules section, and re-emphasise that the all-clear case is COMMENT (not APPROVE).
  3. .github/workflows/shared/review-shared.md — Instruct the launcher agent to forward the same constraint to the subagent's prompt when it spawns it.

Both files are imported at runtime via gh-aw {{#runtime-import}} (verified in the lock files), so no lock-file regeneration is required and the diff is purely the two .md files. gh aw compile was still run to verify there are no warnings.

We deliberately do not add APPROVE to allowed-events — the safe-outputs filter is doing exactly what it was configured to do, and the security model (no automated approvals) should stay intact.

The expert-reviewer agent submitted a submit_pull_request_review with �vent: APPROVE when reviewing PR #8220, which the safe-outputs filter correctly rejected (�llowed-events: [COMMENT, REQUEST_CHANGES]). The rejection caused the whole safe_outputs job to fail, dropping the verdict table along with all inline comments bundled into the review (https://github.com/microsoft/testfx/actions/runs/25860787433/job/75991405833).
The original prompt does say 'Never use APPROVE', but the rule is buried ~630 lines in, after the 21 dimension definitions and the wave-by-wave workflow. The agent missed it.
Reinforce the constraint in three places to make it impossible to miss:
- Add an 'Absolute Rules (read first, must never be violated)' section near the top of .github/agents/expert-reviewer.agent.md that explicitly forbids APPROVE and explains the consequence (entire review dropped).
- Strengthen the existing instruction in Wave 4 step 6 with a stop sign and a back-reference to the Absolute Rules section.
- In shared/review-shared.md, instruct the launcher agent to forward the same constraint to the subagent prompt.
Both files are imported at runtime via gh-aw {{#runtime-import}}, so no lock-file regeneration is required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings May 14, 2026 13:08

CopilotAI 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.

Pull request overview

This PR reinforces the expert-review workflow’s no-automated-approval rule so reviews are submitted only as comments or change requests, preserving the intended safe-outputs security model.

Changes:

  • Adds prominent “Absolute Rules” to the expert reviewer prompt forbidding APPROVE.
  • Strengthens the final submission reminder to use COMMENT for clean reviews.
  • Updates the shared launcher instructions to pass the same constraint to the background reviewer agent.
Show a summary per file
FileDescription
.github/agents/expert-reviewer.agent.mdAdds and reinforces prompt rules preventing APPROVE review submissions.
.github/workflows/shared/review-shared.mdUpdates launcher instructions to remind the subagent about allowed review events.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Expert Review — PR #8221

All 21 review dimensions assessed. No findings.

#DimensionStatus
1Backward Compatibility✅ N/A — no code
2Public API Surface✅ N/A — no code
3Correctness / Logic✅ Fix accurately targets the root cause
4Test Coverage✅ N/A — documentation change
5Performance✅ N/A
6Security✅ Security model preserved — APPROVE remains blocked
7Error Handling✅ N/A
8Null Safety✅ N/A
9Threading / Concurrency✅ N/A
10Resource Management✅ N/A
11Localization✅ N/A
12Logging / Telemetry✅ N/A
13Naming / Style✅ Clear, consistent phrasing
14Code Duplication✅ Intentional repetition across three locations to reinforce the rule
15Complexity✅ Simple, targeted changes
16Documentation✅ This IS the documentation; wording is precise and unambiguous
17Build / CI✅ No lock-file changes required (runtime-import confirmed)
18Dependencies✅ N/A
19Serialization✅ N/A
20Observability✅ N/A
21Design Consistency✅ Follows existing agent-prompt conventions

All clear. The fix is well-structured — placing the constraint in an "Absolute Rules" section at the very top of the file is the right approach for prompt-level constraints that must survive a long context window. The stop-sign (⛔) in Wave 4 and the forwarding instruction in review-shared.md provide effective belt-and-suspenders reinforcement.

Generated by Expert Code Review (on open) for issue #8221 · ● 2.8M

@Evangelink
Amaury Levé (Evangelink) merged commit 443b9b6 into mainMay 14, 2026
28 checks passed
@Evangelink
Amaury Levé (Evangelink) deleted the fix/expert-reviewer-no-approve branch May 14, 2026 13:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Evangelink