Skip to content

[sync] origin/develop <= upstream/develop (a30e4dd9cae8) #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
svelderrainruiz wants to merge 1 commit into from
Closed

[sync] origin/develop <= upstream/develop (a30e4dd9cae8) #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions .github/workflows/issue-snapshot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
node-version: '20'
- name: Sync standing-priority snapshot
env:
AGENT_PRIORITY_UPSTREAM_REPOSITORY: LabVIEW-Community-CI-CD/compare-vi-cli-action
GITHUB_REPOSITORY: ${{ github.repository }}
GITHUB_TOKEN: ${{ github.token }}
GH_TOKEN: ${{ github.token }}
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/validate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,7 @@ jobs:
node-version: '20'
- name: Sync standing-priority snapshot
env:
AGENT_PRIORITY_UPSTREAM_REPOSITORY: LabVIEW-Community-CI-CD/compare-vi-cli-action
GITHUB_REPOSITORY: ${{ github.repository }}
GITHUB_TOKEN: ${{ secrets.GH_TOKEN || secrets.GITHUB_TOKEN }}
GH_TOKEN: ${{ secrets.GH_TOKEN || secrets.GITHUB_TOKEN }}
Expand Down
107 changes: 107 additions & 0 deletions tools/priority/__tests__/copilot-review-gate.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,47 @@ test('copilot-review-gate skips merge-group runs while keeping the required stat
assert.deepEqual(result.report?.reasons, ['merge-group-skip']);
});

test('copilot-review-gate skips throughput fork repos before any live lookup', async () => {
const { runCopilotReviewGate } = await loadModule();
let reviewsCalled = false;
let threadsCalled = false;

const result = await runCopilotReviewGate({
argv: createArgv([
'--event-name',
'pull_request_target',
'--repo',
'svelderrainruiz/compare-vi-cli-action',
'--pr',
'304',
'--head-sha',
'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
'--base-ref',
'develop',
'--draft',
'false',
]),
loadReviewsFn: async () => {
reviewsCalled = true;
return [];
},
loadThreadsFn: async () => {
threadsCalled = true;
return [];
},
writeReportFn: () => 'memory://copilot-review-gate-throughput-fork.json',
appendStepSummaryFn: () => {},
});

assert.equal(result.exitCode, 0);
assert.equal(result.report?.status, 'pass');
assert.equal(result.report?.gateState, 'skipped');
assert.deepEqual(result.report?.reasons, ['throughput-fork-skip']);
assert.equal(result.report?.signals.gateApplies, false);
assert.equal(reviewsCalled, false);
assert.equal(threadsCalled, false);
});

test('copilot-review-gate passes stale but clean follow-up heads after an earlier Copilot review', async () => {
const { runCopilotReviewGate } = await loadModule();
const currentHead = 'cccccccccccccccccccccccccccccccccccccccc';
Expand Down Expand Up @@ -355,6 +396,72 @@ test('copilot-review-gate can evaluate the current-head state from the collected
assert.equal(threadsCalled, false);
});

test('copilot-review-gate reads the signal artifact before throughput-fork preflight skipping', async (t) => {
const { runCopilotReviewGate } = await loadModule();
let reviewsCalled = false;
let threadsCalled = false;
const signalPath = createSignalFixture(t, 'copilot-review-signal-signal-only.json');

const result = await runCopilotReviewGate({
argv: createArgv([
'--event-name',
'pull_request_target',
'--pr',
'885',
'--head-sha',
'9999999999999999999999999999999999999999',
'--base-ref',
'develop',
'--draft',
'false',
'--signal',
signalPath,
]),
readSignalFn: () => ({
schema: 'priority/copilot-review-signal@v1',
repository: 'LabVIEW-Community-CI-CD/compare-vi-cli-action',
pullRequest: {
number: 885,
url: 'https://github.com/LabVIEW-Community-CI-CD/compare-vi-cli-action/pull/885',
draft: false,
headSha: '9999999999999999999999999999999999999999',
baseRef: 'develop',
},
latestCopilotReview: {
id: '15',
state: 'COMMENTED',
commitId: '9999999999999999999999999999999999999999',
submittedAt: '2026-03-08T06:05:00Z',
url: 'https://github.com/example/review/15',
isCurrentHead: true,
bodySummary: 'Current-head Copilot review.',
},
staleReviews: [],
unresolvedThreads: [],
actionableComments: [],
errors: [],
}),
loadReviewsFn: async () => {
reviewsCalled = true;
return [];
},
loadThreadsFn: async () => {
threadsCalled = true;
return [];
},
writeReportFn: () => 'memory://copilot-review-gate-signal-only.json',
appendStepSummaryFn: () => {},
});

assert.equal(result.exitCode, 0);
assert.equal(result.report?.source.mode, 'signal');
assert.equal(result.report?.status, 'pass');
assert.equal(result.report?.gateState, 'ready');
assert.deepEqual(result.report?.reasons, ['current-head-review-clean']);
assert.equal(reviewsCalled, false);
assert.equal(threadsCalled, false);
});

test('copilot-review-gate reports an error when loading reviews fails', async () => {
const { runCopilotReviewGate } = await loadModule();

Expand Down
47 changes: 47 additions & 0 deletions tools/priority/__tests__/standing-priority-workflow-contract.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env node

import test from 'node:test';
import assert from 'node:assert/strict';
import path from 'node:path';
import { readFileSync } from 'node:fs';

const repoRoot = process.cwd();
const canonicalUpstreamSlug = 'LabVIEW-Community-CI-CD/compare-vi-cli-action';

function readRepoFile(relativePath) {
return readFileSync(path.join(repoRoot, relativePath), 'utf8');
}

function escapeRegExp(value) {
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}

test('issue-event standing-priority snapshot resolves fork context through the canonical upstream slug', () => {
const workflow = readRepoFile('.github/workflows/issue-snapshot.yml');
const stepPattern = new RegExp(
[
'- name: Sync standing-priority snapshot',
'[\\s\\S]*?',
`AGENT_PRIORITY_UPSTREAM_REPOSITORY: ${escapeRegExp(canonicalUpstreamSlug)}`,
'[\\s\\S]*?',
'node tools/npm/run-script\\.mjs priority:sync:lane'
].join('')
);

assert.match(workflow, stepPattern);
});

test('validate standing-priority snapshot resolves fork context through the canonical upstream slug', () => {
const workflow = readRepoFile('.github/workflows/validate.yml');
const stepPattern = new RegExp(
[
'- name: Sync standing-priority snapshot',
'[\\s\\S]*?',
`AGENT_PRIORITY_UPSTREAM_REPOSITORY: ${escapeRegExp(canonicalUpstreamSlug)}`,
'[\\s\\S]*?',
'node tools/npm/run-script\\.mjs priority:sync:lane'
].join('')
);

assert.match(workflow, stepPattern);
});
27 changes: 21 additions & 6 deletions tools/priority/copilot-review-gate.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const DEFAULT_GATED_BASE_REFS = ['develop'];
const DEFAULT_POLL_ATTEMPTS = 1;
const DEFAULT_POLL_DELAY_MS = 10000;
const GITHUB_API_URL = 'https://api.github.com';
const CANONICAL_REPOSITORY = 'LabVIEW-Community-CI-CD/compare-vi-cli-action';

const COPILOT_LOGINS = new Set([
'copilot',
Expand Down Expand Up @@ -195,6 +196,11 @@ export function parseRepoSlug(repo) {
return { owner, repo: repoName };
}

function isCanonicalRepository(repo) {
const normalized = normalizeText(repo)?.toLowerCase();
return normalized === CANONICAL_REPOSITORY.toLowerCase();
}
Comment on lines +199 to +202

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

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

isCanonicalRepository() treats a missing/blank repository as non-canonical, which then causes evaluateGateOutcome() to skip the gate with reason throughput-fork-skip. Because the Copilot review signal schema allows repository: null, this can make the gate fail open (skipped) for gated base refs when the repository value is unavailable/corrupt. Consider distinguishing “unknown repository” from “fork repository” (e.g., fail with gateState='error' / a dedicated reason when repository is null) so the queue gate doesn’t get bypassed by missing metadata.

Copilot uses AI. Check for mistakes.

function isCopilotLogin(login) {
const normalized = normalizeText(login)?.toLowerCase();
return normalized ? COPILOT_LOGINS.has(normalized) : false;
Expand Down Expand Up @@ -559,10 +565,11 @@ function evaluateGateOutcome({
}) {
const reasons = [];
const normalizedBaseRef = normalizeBaseRef(pullRequest.baseRef)?.toLowerCase() ?? null;
const canonicalRepository = isCanonicalRepository(repository);
const gateApplies =
eventName !== 'merge_group' &&
pullRequest.draft !== true &&
Boolean(normalizedBaseRef && gatedBaseRefs.includes(normalizedBaseRef));
Boolean(normalizedBaseRef && gatedBaseRefs.includes(normalizedBaseRef) && canonicalRepository);

const summary = {
copilotReviewCount: reviews.length,
Expand Down Expand Up @@ -620,6 +627,9 @@ function evaluateGateOutcome({
} else if (!normalizedBaseRef || !gatedBaseRefs.includes(normalizedBaseRef)) {
gateState = 'skipped';
reasons.push('base-ref-not-gated');
} else if (!canonicalRepository) {
gateState = 'skipped';
reasons.push('throughput-fork-skip');
} else if (errors.length > 0) {
status = 'fail';
gateState = 'error';
Expand Down Expand Up @@ -953,18 +963,24 @@ export async function runCopilotReviewGate({
let report;

try {
const preflightPullRequest = buildPullRequest(options);
const signalPathExists = options.signalPath && existsSync(path.resolve(process.cwd(), options.signalPath));
const signalReport = signalPathExists ? readSignalFn(options.signalPath) : null;
const preflightPullRequest = buildPullRequest(options, signalReport);
Comment on lines +966 to +968

Copilot AI Mar 10, 2026

Copy link

Choose a reason for hiding this comment

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

runCopilotReviewGate() now reads/parses the signal file before deciding shouldSkipWithoutLookup. This means a malformed/partially-written signal JSON can fail the entire gate run even when it would otherwise be skipped (draft PR, non-gated base ref, merge_group, etc.). To keep the skip path resilient, consider only reading the signal early when it’s actually needed for preflight (e.g., when --repo/PR metadata is missing), or catch JSON parse errors and treat the signal as unavailable when the run is going to be skipped.

Copilot uses AI. Check for mistakes.
const preflightBaseRef = normalizeBaseRef(preflightPullRequest.baseRef)?.toLowerCase() ?? null;
const preflightRepository =
normalizeText(signalReport?.repository) ??
normalizeText(options.repo);
const shouldSkipWithoutLookup =
options.eventName === 'merge_group' ||
preflightPullRequest.draft === true ||
!preflightBaseRef ||
!options.gatedBaseRefs.includes(preflightBaseRef);
!options.gatedBaseRefs.includes(preflightBaseRef) ||
(preflightRepository !== null && !isCanonicalRepository(preflightRepository));

if (shouldSkipWithoutLookup) {
report = evaluateGateOutcome({
eventName: options.eventName,
repository: normalizeText(options.repo),
repository: preflightRepository,
sourceMode: options.eventName === 'merge_group' ? 'merge-group' : 'metadata',
pullRequest: preflightPullRequest,
reviews: [],
Expand All @@ -973,8 +989,7 @@ export async function runCopilotReviewGate({
gatedBaseRefs: options.gatedBaseRefs,
now,
});
} else if (options.signalPath && existsSync(path.resolve(process.cwd(), options.signalPath))) {
const signalReport = readSignalFn(options.signalPath);
} else if (signalReport) {
report = buildReportFromSignal(options, signalReport, now);
} else {
const reviews = await loadReviewsFn(options);
Expand Down
Loading