mirror of
https://github.com/DingTalk-Real-AI/dingtalk-workspace-cli.git
synced 2026-09-29 16:57:48 +08:00
fix(ci): retry admitted-merge PR discovery past association-index lag
Protected-main pushes fire within seconds of the merge, before GitHub's commit-to-PR association index necessarily lists the fresh merge commit. Both production attempts since the reuse mechanism landed fell back to the complete main suite with "expected one associated merged PR, found 0" even though the PR records already bound the exact merge identity. Discovery now re-polls on a bounded budget (twelve attempts five seconds apart by default, tunable through DWS_ADMITTED_MERGE_RETRY_*) and also consults the most recently updated closed main PRs under the identical exact filter, since the merge transaction records merge_commit_sha on the PR before the push event fires. Only zero-match discovery retries; ambiguous results still throw immediately and an exhausted budget keeps the complete protected-main suite, so fail-closed semantics are unchanged.
This commit is contained in:
+72
-12
@@ -528,16 +528,7 @@ jobs:
|
||||
throw new Error('target is not an exact two-parent merge from push before');
|
||||
}
|
||||
const admittedHeadSha = targetCommit.parents[1].sha;
|
||||
const associatedPulls = await github.paginate(
|
||||
github.rest.pulls.listPullRequestsAssociatedWithCommit,
|
||||
{
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
commit_sha: expectedAfter,
|
||||
per_page: 100,
|
||||
},
|
||||
);
|
||||
const admittedPulls = associatedPulls.filter((pull) =>
|
||||
const matchesAdmittedMerge = (pull) =>
|
||||
pull.state === 'closed' &&
|
||||
typeof pull.merged_at === 'string' &&
|
||||
pull.merged_at.length > 0 &&
|
||||
@@ -550,11 +541,80 @@ jobs:
|
||||
typeof pull.head?.ref === 'string' &&
|
||||
pull.head.ref.length > 0 &&
|
||||
typeof pull.head?.repo?.full_name === 'string' &&
|
||||
pull.head.repo.full_name.length > 0,
|
||||
pull.head.repo.full_name.length > 0;
|
||||
const findAdmittedPulls = async () => {
|
||||
const associatedPulls = await github.paginate(
|
||||
github.rest.pulls.listPullRequestsAssociatedWithCommit,
|
||||
{
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
commit_sha: expectedAfter,
|
||||
per_page: 100,
|
||||
},
|
||||
);
|
||||
const matched = associatedPulls.filter(matchesAdmittedMerge);
|
||||
if (matched.length > 0) {
|
||||
return matched;
|
||||
}
|
||||
// A protected-main push fires within seconds of the
|
||||
// merge, before the commit-to-PR association index
|
||||
// necessarily lists the fresh merge commit. The merge
|
||||
// transaction has already recorded merge_commit_sha on
|
||||
// the PR itself, so the most recently updated closed
|
||||
// PRs are a second discovery source under the identical
|
||||
// exact filter. One page only: the just-merged PR is
|
||||
// always among the latest updated closed main PRs.
|
||||
try {
|
||||
const {data: recentlyClosedPulls} =
|
||||
await github.rest.pulls.list({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
state: 'closed',
|
||||
base: 'main',
|
||||
sort: 'updated',
|
||||
direction: 'desc',
|
||||
per_page: 100,
|
||||
});
|
||||
return (recentlyClosedPulls || []).filter(matchesAdmittedMerge);
|
||||
} catch (error) {
|
||||
core.warning(
|
||||
`closed-PR fallback discovery failed: ${error.message}`,
|
||||
);
|
||||
return [];
|
||||
}
|
||||
};
|
||||
// Bounded retry so discovery can wait out association-index
|
||||
// lag instead of failing closed on the first snapshot.
|
||||
// Only zero matches retry; ambiguity throws immediately.
|
||||
const retryIntervalMs = Math.max(
|
||||
0,
|
||||
Number.parseInt(
|
||||
process.env.DWS_ADMITTED_MERGE_RETRY_INTERVAL_MS || '5000',
|
||||
10,
|
||||
) || 0,
|
||||
);
|
||||
const retryAttempts = Math.max(
|
||||
1,
|
||||
Number.parseInt(
|
||||
process.env.DWS_ADMITTED_MERGE_RETRY_ATTEMPTS || '12',
|
||||
10,
|
||||
) || 1,
|
||||
);
|
||||
let admittedPulls = await findAdmittedPulls();
|
||||
let discoveryAttempts = 1;
|
||||
while (admittedPulls.length === 0 && discoveryAttempts < retryAttempts) {
|
||||
discoveryAttempts += 1;
|
||||
if (retryIntervalMs > 0) {
|
||||
await new Promise((resolve) => {
|
||||
setTimeout(resolve, retryIntervalMs);
|
||||
});
|
||||
}
|
||||
admittedPulls = await findAdmittedPulls();
|
||||
}
|
||||
if (admittedPulls.length !== 1) {
|
||||
throw new Error(
|
||||
`expected one associated merged PR, found ${admittedPulls.length}`,
|
||||
`expected one associated merged PR, found ${admittedPulls.length} ` +
|
||||
`after ${discoveryAttempts} discovery attempt(s)`,
|
||||
);
|
||||
}
|
||||
const admittedPull = admittedPulls[0];
|
||||
|
||||
@@ -155,6 +155,18 @@ The classifier fails closed unless it can prove every one of these facts:
|
||||
published SHA-256 digest and the same head SHA; the archive's admission
|
||||
manifest independently binds the run ID, head SHA, and `full` profile kind.
|
||||
|
||||
Merged-PR discovery tolerates GitHub's commit-to-PR association index lag. A
|
||||
protected-main push fires within seconds of the merge, and a single-shot
|
||||
association lookup was observed returning zero matches for eligible merges in
|
||||
production, silently forcing the complete suite. The classifier therefore
|
||||
re-polls discovery on a bounded budget (`DWS_ADMITTED_MERGE_RETRY_ATTEMPTS`
|
||||
attempts, `DWS_ADMITTED_MERGE_RETRY_INTERVAL_MS` apart; twelve attempts at
|
||||
five seconds by default) and additionally consults the most recently updated
|
||||
closed `main` PRs under the identical exact filter, because the merge
|
||||
transaction records `merge_commit_sha` on the PR before the push event fires.
|
||||
Only zero-match discovery retries; an ambiguous or mismatched result throws
|
||||
immediately, and an exhausted budget keeps the complete protected-main suite.
|
||||
|
||||
When those facts hold, `Lint` publishes the bound PR head, run, artifact, and
|
||||
digest. The existing `coverage-main-metadata` job downloads the artifact by
|
||||
numeric ID, revalidates its API identity, verifies the downloaded archive's
|
||||
|
||||
@@ -34,6 +34,9 @@ func TestAdmittedMergeClassifierFailsClosed(t *testing.T) {
|
||||
admitted string
|
||||
}{
|
||||
{name: "exact evidence is reused", scenario: "success", admitted: "true"},
|
||||
{name: "association index lag recovers on retry", scenario: "association-index-lag", admitted: "true"},
|
||||
{name: "cold association index recovers via closed-PR discovery", scenario: "association-index-cold", admitted: "true"},
|
||||
{name: "unresolvable merged PR recomputes full suite", scenario: "merged-pr-unresolvable", admitted: "false"},
|
||||
{name: "comparison API failure recomputes full suite", scenario: "compare-error", admitted: "false"},
|
||||
{name: "predecessor checks API failure recomputes full suite", scenario: "predecessor-check-error", admitted: "false"},
|
||||
{name: "failed predecessor admission recomputes full suite", scenario: "predecessor-check-failure", admitted: "false"},
|
||||
@@ -150,6 +153,11 @@ func runAdmittedMergeClassifier(t *testing.T, node, classifier, scenario string)
|
||||
t.Helper()
|
||||
const harnessPrefix = `
|
||||
const scenario = process.argv[2];
|
||||
// Keep the production discovery retry loop fast inside the harness; the
|
||||
// production defaults stay in the classifier script itself.
|
||||
process.env.DWS_ADMITTED_MERGE_RETRY_INTERVAL_MS = '1';
|
||||
process.env.DWS_ADMITTED_MERGE_RETRY_ATTEMPTS = '5';
|
||||
let associationCalls = 0;
|
||||
const before = 'b'.repeat(40);
|
||||
const after = 'a'.repeat(40);
|
||||
const head = 'c'.repeat(40);
|
||||
@@ -257,7 +265,17 @@ const endpoints = {
|
||||
}]}),
|
||||
},
|
||||
pulls: {
|
||||
listPullRequestsAssociatedWithCommit: async () => ({data: [pull]}),
|
||||
listPullRequestsAssociatedWithCommit: async () => {
|
||||
if (scenario === 'association-index-lag') {
|
||||
associationCalls += 1;
|
||||
return {data: associationCalls > 2 ? [pull] : []};
|
||||
}
|
||||
if (scenario === 'association-index-cold' || scenario === 'merged-pr-unresolvable') {
|
||||
return {data: []};
|
||||
}
|
||||
return {data: [pull]};
|
||||
},
|
||||
list: async () => ({data: scenario === 'association-index-cold' ? [pull] : []}),
|
||||
},
|
||||
checks: {
|
||||
listForRef: async ({ref}) => {
|
||||
|
||||
Reference in New Issue
Block a user