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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion .github/workflows/reviewrouter-execution-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ on:
required: false
type: string
default: ''
review_head_sha:
required: false
type: string
default: ''
review_app_client_id:
required: false
type: string
Expand Down Expand Up @@ -213,8 +217,15 @@ jobs:
echo "ReviewRouter skipped this fork pull request because secret-backed provider execution is disabled by default."
fi

- name: Checkout exact T0 review revision
if: ${{ inputs.review_action_lane == 't0' && steps.runtime.outputs.can_run == 'true' }}
uses: actions/checkout@v6
with:
ref: ${{ inputs.review_head_sha }}
persist-credentials: false

- name: Checkout pull request code
if: ${{ steps.runtime.outputs.can_run == 'true' }}
if: ${{ inputs.review_action_lane == 'legacy' && steps.runtime.outputs.can_run == 'true' }}
Comment on lines +220 to +228

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect how review_head_sha is supplied by callers of the reusable workflows.
rg -nP -C3 'review_head_sha' .github/workflows

Repository: 777genius/review-router

Length of output: 5584


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== relevant files =="
git ls-files .github/workflows/reviewrouter-execution-reusable.yml .github/workflows/reviewrouter-reusable.yml

echo
echo "== reviewer execution workflow outline/context =="
wc -l .github/workflows/reviewrouter-execution-reusable.yml
sed -n '1,90p' .github/workflows/reviewrouter-execution-reusable.yml
sed -n '200,240p' .github/workflows/reviewrouter-execution-reusable.yml
sed -n '320,345p' .github/workflows/reviewrouter-execution-reusable.yml

echo
echo "== reusable workflow inputs and runtime env =="
sed -n '1,60p' .github/workflows/reviewrouter-reusable.yml
sed -n '90,130p' .github/workflows/reviewrouter-reusable.yml
sed -n '140,190p' .github/workflows/reviewrouter-reusable.yml

echo
echo "== usages of reviewer execution workflow in repo (text diff context) =="
python3 - <<'PY'
from pathlib import Path
import re
for p in sorted(Path('.github').rglob('*.yml')) + sorted(Path('.github').rglob('*.yaml')):
    b = p.read_text(errors='ignore')
    if 'reviewrouter-execution-reusable.yml' in b:
        lines=b.splitlines()
        print(f'--- {p} ---')
        for i,l in enumerate(lines,1):
            if 'reviewrouter-execution-reusable.yml' in l or 'review_head_sha' in l:
                a=max(1,i-4); z=min(len(lines),i+4)
                for j in range(a,z+1):
                    print(f'{j:4}: {lines[j-1]}')
                print()
PY

Repository: 777genius/review-router

Length of output: 16319


Use the same SHA for the T0 checkout and runtime input.

The T0 tree is checked out at inputs.review_head_sha, but the runtime input prefers the live github.event.pull_request.head.sha. If a PR advances after the T0 workflow is queued, inputs.review_head_sha stays at the older durable value while the runtime can receive/validate the newer head, breaking the exact-head durability guarantee. Align the checkout ref with the runtime input precedence, or pass github.event.pull_request.head.sha to both when operating on a live PR.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/reviewrouter-execution-reusable.yml around lines 220 -
228, Align the T0 checkout step named “Checkout exact T0 review revision” with
the runtime input’s SHA precedence: use the same resolved commit value for both
checkout ref and runtime validation/input, preferring the live pull request head
SHA when operating on a live PR and otherwise using inputs.review_head_sha.
Preserve exact-head consistency so checkout and runtime never target different
revisions.

uses: actions/checkout@v6
with:
persist-credentials: false
Expand Down Expand Up @@ -325,6 +336,7 @@ jobs:
INPUT_OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }}
REVIEW_ROUTER_LEDGER_KEY: ${{ secrets.REVIEW_ROUTER_LEDGER_KEY }}
PR_NUMBER: ${{ github.event.pull_request.number || inputs.pr_number }}
REVIEW_HEAD_SHA: ${{ github.event.pull_request.head.sha || inputs.review_head_sha }}
REVIEW_ROUTER_MEMORY_ENABLED: 'true'
REVIEW_ROUTER_MEMORY_PROTOCOL_VERSION: '1'
REVIEW_ROUTER_MEMORY_BUNDLE_ENDPOINT: /api/action/v1/memory
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/reviewrouter-reusable.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ on:
required: false
type: string
default: ''
review_head_sha:
description: 'Expected pull request head for durable workflow_dispatch review requests'
required: false
type: string
default: ''
review_app_client_id:
description: 'Optional user-owned GitHub App client ID for static-mode bot comments'
required: false
Expand Down Expand Up @@ -97,6 +102,7 @@ jobs:
env:
REVIEW_ACTION_V2_MODE: ${{ inputs.review_action_v2_mode }}
PROVIDER_INSTANCE_ID: ${{ inputs.provider_instance_id }}
REVIEW_HEAD_SHA: ${{ inputs.review_head_sha }}
WORKFLOW_SCHEMA_VERSION: ${{ inputs.workflow_schema_version }}
RUNTIME_CONFIG_MODE: ${{ inputs.runtime_config_mode }}
run: |
Expand All @@ -113,6 +119,10 @@ jobs:
echo "::error::provider_instance_id is required for the t0 lane."
exit 1
fi
if ! [[ "$REVIEW_HEAD_SHA" =~ ^[a-fA-F0-9]{40}$ ]]; then
echo "::error::review_head_sha must be a 40-character commit SHA for the t0 lane."
exit 1
fi
if ! [[ "$WORKFLOW_SCHEMA_VERSION" =~ ^[1-9][0-9]*$ ]]; then
echo "::error::workflow_schema_version must be a positive integer."
exit 1
Expand All @@ -139,6 +149,7 @@ jobs:
runtime_config_mode: ${{ inputs.runtime_config_mode }}
static_runtime_env_json: ${{ inputs.static_runtime_env_json }}
pr_number: ${{ inputs.pr_number }}
review_head_sha: ${{ inputs.review_head_sha }}
review_app_client_id: ''
review_app_repository: ''
provider_instance_id: ${{ inputs.provider_instance_id }}
Expand Down Expand Up @@ -170,6 +181,7 @@ jobs:
runtime_config_mode: ${{ inputs.runtime_config_mode }}
static_runtime_env_json: ${{ inputs.static_runtime_env_json }}
pr_number: ${{ inputs.pr_number }}
review_head_sha: ''
review_app_client_id: ${{ inputs.review_app_client_id }}
review_app_repository: ${{ inputs.review_app_repository }}
provider_instance_id: ''
Expand Down
47 changes: 47 additions & 0 deletions __tests__/unit/codex-oauth/action-mode.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import {
CODEX_OAUTH_ROTATING_MODE,
readPullRequestEvent,
shouldEnterCodexOAuthRotatingAction,
} from '../../../src/codex-oauth/action';

Expand Down Expand Up @@ -30,4 +34,47 @@ describe('Codex OAuth rotating action mode', () => {
})
).toBe(false);
});

it('binds workflow_dispatch reviews to the server-selected PR and head', () => {
const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'rr-dispatch-'));
const eventPath = path.join(directory, 'event.json');
fs.writeFileSync(
eventPath,
JSON.stringify({
repository: { full_name: '777genius/agent-teams-ai' },
inputs: {
pr_number: '252',
review_head_sha: 'a'.repeat(40),
},
})
);
const previous = {
eventPath: process.env.GITHUB_EVENT_PATH,
eventName: process.env.GITHUB_EVENT_NAME,
repository: process.env.GITHUB_REPOSITORY,
};
process.env.GITHUB_EVENT_PATH = eventPath;
process.env.GITHUB_EVENT_NAME = 'workflow_dispatch';
process.env.GITHUB_REPOSITORY = '777genius/agent-teams-ai';

try {
expect(readPullRequestEvent()).toEqual({
repository: '777genius/agent-teams-ai',
number: 252,
headSha: 'a'.repeat(40),
headRef: '',
eventName: 'workflow_dispatch',
});
} finally {
restoreEnv('GITHUB_EVENT_PATH', previous.eventPath);
restoreEnv('GITHUB_EVENT_NAME', previous.eventName);
restoreEnv('GITHUB_REPOSITORY', previous.repository);
fs.rmSync(directory, { recursive: true, force: true });
}
});
});

function restoreEnv(name: string, value: string | undefined): void {
if (value === undefined) delete process.env[name];
else process.env[name] = value;
}
78 changes: 78 additions & 0 deletions __tests__/unit/control-plane/review-request.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import {
ControlPlaneManualReviewRequestClient,
ManualReviewRequestAvailability,
} from '../../../src/control-plane/review-request';

const applied = {
status: 'applied' as const,
apiUrl: 'https://api.reviewrouter.site',
actionVersion: '1.0.0',
configVersion: 1,
sessionToken: 'session-token',
};

describe('ControlPlaneManualReviewRequestClient', () => {
it('treats runtime-config fallback as unavailable rather than legacy-safe', () => {
const client = new ControlPlaneManualReviewRequestClient({
status: 'fallback',
reason: 'network_error',
});
expect(client.availability()).toBe(
ManualReviewRequestAvailability.Unavailable
);
});

it('does not treat repository_not_registered as an unsupported endpoint', async () => {
const client = new ControlPlaneManualReviewRequestClient(
applied,
jest.fn(
async () =>
new Response(
JSON.stringify({ error: { code: 'repository_not_registered' } }),
{ status: 404, headers: { 'content-type': 'application/json' } }
)
) as typeof fetch
);

await expect(client.request(command())).rejects.toThrow(
'manual_review_request_failed:404:repository_not_registered'
);
});

it.each([
[{ error: { code: 'review_request_intent_disabled' } }],
[
{
error: 'Not Found',
message: 'Route POST:/api/action/v1/review-requests/manual not found',
},
],
])(
'allows legacy fallback only for an explicitly unsupported endpoint',
async (body) => {
const client = new ControlPlaneManualReviewRequestClient(
applied,
jest.fn(
async () =>
new Response(JSON.stringify(body), {
status: 404,
headers: { 'content-type': 'application/json' },
})
) as typeof fetch
);

await expect(client.request(command())).resolves.toEqual({
status: 'unsupported',
});
}
);
});

function command() {
return {
pullRequestNumber: 252,
expectedHeadSha: 'a'.repeat(40),
sourceId: 'manual-comment:1',
commandKind: 'review' as const,
};
}
152 changes: 152 additions & 0 deletions __tests__/unit/github/interaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import {
ActionMemoryInteractionPort,
ActionMemoryMutationResponse,
} from '../../../src/control-plane/memory';
import {
ManualReviewRequestAvailability,
type ManualReviewRequestPort,
type ManualReviewRequestCommandKind,
} from '../../../src/control-plane/review-request';

function writeEvent(payload: unknown): string {
const file = path.join(
Expand Down Expand Up @@ -118,6 +123,28 @@ class CapturingMemoryClient implements ActionMemoryInteractionPort {
}
}

class CapturingReviewRequestClient implements ManualReviewRequestPort {
public readonly requests: Array<{
readonly pullRequestNumber: number;
readonly expectedHeadSha: string;
readonly sourceId: string;
readonly commandKind: ManualReviewRequestCommandKind;
}> = [];

constructor(
private readonly state = ManualReviewRequestAvailability.Available
) {}

availability(): ManualReviewRequestAvailability {
return this.state;
}

async request(input: (typeof this.requests)[number]) {
this.requests.push(input);
return { status: 'queued' as const };
}
}

describe('ReviewInteractionHandler', () => {
const originalEnv = { ...process.env };

Expand Down Expand Up @@ -185,6 +212,131 @@ describe('ReviewInteractionHandler', () => {
);
});

it('creates a revision-aware manual intent without rerunning an old workflow attempt', async () => {
const { client, octokit } = makeClient();
const reviewRequests = new CapturingReviewRequestClient();
process.env.GITHUB_EVENT_PATH = writeEvent({
comment: {
id: 11,
in_reply_to_id: 10,
body: '/rr skip verified',
user: { login: 'maintainer' },
},
pull_request: {
number: 123,
head: { sha: 'a'.repeat(40), repo: { fork: false } },
user: { login: 'author' },
},
});
octokit.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({
data: { permission: 'write', role_name: 'maintain' },
});

await new ReviewInteractionHandler(
client,
new ReviewLedger(client, 'test-secret'),
undefined,
client,
undefined,
reviewRequests
).execute();

expect(reviewRequests.requests).toEqual([
{
pullRequestNumber: 123,
expectedHeadSha: 'a'.repeat(40),
sourceId: 'review-comment:11',
commandKind: 'skip',
},
]);
expect(octokit.rest.actions.listWorkflowRunsForRepo).not.toHaveBeenCalled();
expect(octokit.rest.actions.reRunWorkflowFailedJobs).not.toHaveBeenCalled();
});

it('queues a top-level /rr review as a distinct same-head request', async () => {
const { client, octokit } = makeClient();
const reviewRequests = new CapturingReviewRequestClient();
process.env.GITHUB_EVENT_PATH = writeEvent({
comment: {
id: 22,
body: '/rr review',
user: { login: 'maintainer' },
},
issue: { number: 123, pull_request: {} },
});
octokit.rest.pulls.get.mockResolvedValue({
data: {
number: 123,
head: { sha: 'b'.repeat(40), repo: { fork: false } },
user: { login: 'author' },
},
});
octokit.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({
data: { permission: 'write', role_name: 'maintain' },
});

await new ReviewInteractionHandler(
client,
new ReviewLedger(client, 'test-secret'),
undefined,
client,
undefined,
reviewRequests
).execute();

expect(reviewRequests.requests).toEqual([
{
pullRequestNumber: 123,
expectedHeadSha: 'b'.repeat(40),
sourceId: 'manual-comment:22',
commandKind: 'review',
},
]);
expect(octokit.rest.actions.listWorkflowRunsForRepo).not.toHaveBeenCalled();
});

it('does not rerun a legacy attempt when control-plane availability is ambiguous', async () => {
const { client, octokit } = makeClient();
const reviewRequests = new CapturingReviewRequestClient(
ManualReviewRequestAvailability.Unavailable
);
process.env.GITHUB_EVENT_PATH = writeEvent({
comment: {
id: 11,
in_reply_to_id: 10,
body: '/rr skip verified',
user: { login: 'maintainer' },
},
pull_request: {
number: 123,
head: { sha: 'a'.repeat(40), repo: { fork: false } },
user: { login: 'author' },
},
});
octokit.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({
data: { permission: 'write', role_name: 'maintain' },
});

await new ReviewInteractionHandler(
client,
new ReviewLedger(client, 'test-secret'),
undefined,
client,
undefined,
reviewRequests
).execute();

expect(octokit.rest.actions.listWorkflowRunsForRepo).not.toHaveBeenCalled();
expect(octokit.rest.actions.reRunWorkflowFailedJobs).not.toHaveBeenCalled();
expect(octokit.rest.issues.createComment).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.stringContaining(
'did not rerun an older workflow attempt'
),
})
);
});

it('uses the workflow token client for rerunning checks when comments use an App token', async () => {
const { client: commentClient, octokit: commentOctokit } = makeClient();
const { client: actionsClient, octokit: actionsOctokit } = makeClient();
Expand Down
Loading
Loading