Skip to content

Commit 31252a3

Browse files
committed
Fix orchestrator human review detection
1 parent 7727aae commit 31252a3

2 files changed

Lines changed: 43 additions & 9 deletions

File tree

.github/scripts/orchestrator.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ module.exports = async ({ github, context, core }) => {
7676
return !labels.includes('auto-merge');
7777
}
7878

79+
function isBotUser(user) {
80+
return !!user && ((user.type || '').toLowerCase() === 'bot' ||
81+
(user.login || '').includes('[bot]') ||
82+
(user.login || '').startsWith('app/'));
83+
}
84+
7985
async function mergePR(prNumber) {
8086
core.info(`Merging PR #${prNumber}...`);
8187
const pr = await github.rest.pulls.get({
@@ -616,13 +622,8 @@ module.exports = async ({ github, context, core }) => {
616622
if (task.phase !== 'HUMAN_REVIEW') return;
617623
if (task.pr !== context.payload.pull_request.number) return;
618624

619-
let botLogin = '';
620-
try {
621-
const { data: user } = await github.rest.users.getAuthenticated();
622-
botLogin = user.login;
623-
} catch (err) {}
624-
625-
if (context.payload.review.user.login === botLogin) {
625+
const reviewer = context.payload.review.user || {};
626+
if (isBotUser(reviewer)) {
626627
core.info("Ignoring review from bot identity.");
627628
return;
628629
}

.github/scripts/tests/orchestrator.test.js

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ function createMocks(eventName = 'issue_comment', commentBody = '/orchestrate')
1010
error: (msg) => logs.error.push(msg)
1111
};
1212

13-
const calls = { getIssue: [], updateIssue: [], createComment: [], listComments: [], addLabels: [], removeLabel: [] };
13+
const calls = { getIssue: [], updateIssue: [], createComment: [], listComments: [], addLabels: [], removeLabel: [], mergePR: [] };
1414
const updateIssueDetails = [];
1515
let masterIssueBody = `- [ ] #2\n- [ ] #3`;
1616
let currentTaskData = { labels: [], state: 'open' };
@@ -49,7 +49,9 @@ function createMocks(eventName = 'issue_comment', commentBody = '/orchestrate')
4949
get: async () => {
5050
return { data: prData };
5151
},
52-
merge: async () => {}
52+
merge: async ({ pull_number }) => {
53+
calls.mergePR.push(pull_number);
54+
}
5355
},
5456
git: {
5557
deleteRef: async () => {}
@@ -69,6 +71,14 @@ function createMocks(eventName = 'issue_comment', commentBody = '/orchestrate')
6971
context.payload = {
7072
workflow_run: { id: 100, display_title: 'Issue #2 - Title', head_commit: { message: 'Issue #2 - Title' }, conclusion: 'success' }
7173
};
74+
} else if (eventName === 'pull_request_review') {
75+
context.payload = {
76+
pull_request: { number: 99 },
77+
review: {
78+
user: { login: 'LSantha', type: 'User' },
79+
state: 'approved'
80+
}
81+
};
7282
}
7383

7484
return { core, github, context, logs, calls, updateIssueDetails, setMasterBody: (b) => masterIssueBody = b, setTaskData: (d) => currentTaskData = d };
@@ -124,6 +134,29 @@ test('orchestrator.js test suite', async (t) => {
124134
assert.ok(updateIssueDetails.some(u => u.issue_number === 1 && u.body.includes('HUMAN_REVIEW')));
125135
});
126136

137+
await t.test('Human approved PR review advances to merge', async () => {
138+
const { core, github, context, calls, setMasterBody } = createMocks('pull_request_review');
139+
140+
setMasterBody(`<!-- ORCHESTRATOR_STATE:\n{ "status": "IN_PROGRESS", "current_task": { "issue": 2, "pr": 99, "phase": "HUMAN_REVIEW", "turn": 0, "max_turns": 3, "retries": 0 }, "queue": [3], "completed": [], "failed": [], "order": [2, 3] }\n-->`);
141+
142+
await runOrchestrator({ github, context, core });
143+
144+
assert.deepStrictEqual(calls.mergePR, [99]);
145+
assert.ok(calls.createComment.some(c => c.issue_number === 3 && c.body.includes('/oc Please proceed')));
146+
});
147+
148+
await t.test('Bot PR review is ignored', async () => {
149+
const { core, github, context, calls, setMasterBody } = createMocks('pull_request_review');
150+
context.payload.review.user = { login: 'opencode-agent[bot]', type: 'Bot' };
151+
152+
setMasterBody(`<!-- ORCHESTRATOR_STATE:\n{ "status": "IN_PROGRESS", "current_task": { "issue": 2, "pr": 99, "phase": "HUMAN_REVIEW", "turn": 0, "max_turns": 3, "retries": 0 }, "queue": [3], "completed": [], "failed": [], "order": [2, 3] }\n-->`);
153+
154+
await runOrchestrator({ github, context, core });
155+
156+
assert.strictEqual(calls.mergePR.length, 0);
157+
assert.strictEqual(calls.updateIssue.length, 0);
158+
});
159+
127160
await t.test('Workflow run handles task failure (retries)', async () => {
128161
const { core, github, context, calls, setMasterBody } = createMocks('workflow_run');
129162
context.payload.workflow_run.conclusion = 'failure';

0 commit comments

Comments
 (0)