mirror of
https://github.com/RayLabsHQ/gitea-mirror.git
synced 2026-08-11 03:12:54 +02:00
fix: prevent duplicate milestones & labels on every sync (#299)
Paginate /milestones and /labels with Link header + X-Total-Count fallback, and pass state=all on milestones so closed milestones aren't re-POSTed on every sync. Caches newly-created items in the in-memory dedup set.
This commit is contained in:
Sean Mousseau
@@ -0,0 +1,200 @@
|
||||
/**
|
||||
* Regression test for duplicate milestone & label creation on every sync.
|
||||
*
|
||||
* Observed in production (May 2026): a Gitea instance accumulated
|
||||
* 11,847 duplicate closed milestones across 4 mirrored repos after only
|
||||
* a handful of scheduled syncs. Two compounding bugs in
|
||||
* `mirrorGitRepoMilestonesToGitea`:
|
||||
*
|
||||
* (1) The existing-milestones GET to Gitea did NOT pass `state=all`.
|
||||
* Gitea's /milestones endpoint defaults to `state=open`, so the
|
||||
* `existingMilestones` Set never contained any closed milestone
|
||||
* title. Every closed GitHub milestone was misclassified as
|
||||
* missing and re-POSTed on every sync.
|
||||
*
|
||||
* (2) The existing-milestones GET was a single unpaginated call.
|
||||
* Gitea caps response size at `[api].MAX_RESPONSE_ITEMS`
|
||||
* (default 50), so any repo with more milestones than that
|
||||
* silently truncates even when (1) is fixed.
|
||||
*
|
||||
* `mirrorGitRepoLabelsToGitea` has the same pagination bug (2). It
|
||||
* doesn't have bug (1) because /labels has no state filter, and it
|
||||
* hadn't yet shown duplicates in production only because no mirrored
|
||||
* repo had crossed 50 distinct labels — but it would the moment one
|
||||
* did.
|
||||
*
|
||||
* These tests assert on the *structure* of the source rather than
|
||||
* invoking the functions, because behavioral tests for the metadata
|
||||
* pipeline require heavy module mocks that pollute other test files
|
||||
* (bun's mock.module is process-wide). Same convention as
|
||||
* `gitea-issue-dedup-on-retry.test.ts` and
|
||||
* `gitea-mirror-failure-recovery.test.ts`.
|
||||
*/
|
||||
import { describe, test, expect } from "bun:test";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
const SOURCE = readFileSync(join(import.meta.dir, "gitea.ts"), "utf8");
|
||||
|
||||
/**
|
||||
* Locate the body of a function declaration by name. Walks from the
|
||||
* declaration, balances parens to skip the parameter list (which can
|
||||
* contain destructured object literals with their own braces), then
|
||||
* finds the body's opening brace and its matching close.
|
||||
*/
|
||||
function extractFunctionBody(source: string, declarationStart: RegExp): string {
|
||||
const match = source.match(declarationStart);
|
||||
if (!match) {
|
||||
throw new Error(`Could not locate declaration ${declarationStart}`);
|
||||
}
|
||||
let i = match.index! + match[0].length;
|
||||
while (i < source.length && source[i] !== "(") i++;
|
||||
if (source[i] !== "(") {
|
||||
throw new Error(`No '(' after ${declarationStart}`);
|
||||
}
|
||||
let parenDepth = 0;
|
||||
for (; i < source.length; i++) {
|
||||
if (source[i] === "(") parenDepth++;
|
||||
else if (source[i] === ")") {
|
||||
parenDepth--;
|
||||
if (parenDepth === 0) {
|
||||
i++;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
while (i < source.length && source[i] !== "{") i++;
|
||||
if (source[i] !== "{") {
|
||||
throw new Error(`No body '{' for ${declarationStart}`);
|
||||
}
|
||||
let braceDepth = 0;
|
||||
const startIdx = i;
|
||||
for (; i < source.length; i++) {
|
||||
if (source[i] === "{") braceDepth++;
|
||||
else if (source[i] === "}") {
|
||||
braceDepth--;
|
||||
if (braceDepth === 0) {
|
||||
return source.slice(startIdx, i + 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
throw new Error(`Unterminated body for ${declarationStart}`);
|
||||
}
|
||||
|
||||
describe("milestone dedup on sync", () => {
|
||||
const body = extractFunctionBody(
|
||||
SOURCE,
|
||||
/export async function mirrorGitRepoMilestonesToGitea\b/
|
||||
);
|
||||
|
||||
test("body contains the per-milestone create branch we expect to guard", () => {
|
||||
// Sanity check: anchor the rest of this suite to the right path.
|
||||
expect(
|
||||
body.includes("existingMilestones"),
|
||||
"expected the existingMilestones set/map used for dedup"
|
||||
).toBe(true);
|
||||
expect(
|
||||
body.match(
|
||||
/await httpPost\(\s*`\$\{config\.giteaConfig\.url\}\/api\/v1\/repos\/\$\{giteaOwner\}\/\$\{repoName\}\/milestones`/
|
||||
),
|
||||
"expected the create-milestone httpPost call"
|
||||
).toBeTruthy();
|
||||
});
|
||||
|
||||
test("existing-milestones GET must include state=all", () => {
|
||||
// Without state=all, Gitea returns only open milestones, so every
|
||||
// closed GitHub milestone is misclassified as missing and re-POSTed
|
||||
// on every sync. This was the root cause of the production blowup.
|
||||
const getMatch = body.match(
|
||||
/httpGet\(\s*`\$\{config\.giteaConfig\.url\}\/api\/v1\/repos\/\$\{giteaOwner\}\/\$\{repoName\}\/milestones\?[^`]*`/
|
||||
);
|
||||
expect(
|
||||
getMatch,
|
||||
"expected a milestones httpGet call with a query string"
|
||||
).toBeTruthy();
|
||||
expect(
|
||||
/state=all/.test(getMatch![0]),
|
||||
"the existing-milestones GET must pass state=all (Gitea defaults to state=open)"
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test("existing-milestones GET must paginate with both Link and X-Total-Count fallback", () => {
|
||||
// Even with state=all, a single unpaginated call only ever sees
|
||||
// the first 50 milestones (Gitea's MAX_RESPONSE_ITEMS cap).
|
||||
//
|
||||
// Gitea's /milestones endpoint does NOT emit a Link header — only
|
||||
// `X-Total-Count`. A strict Link-only check terminates after page 1
|
||||
// and re-POSTs every milestone past index 50 on every sync (the
|
||||
// 9-milestone leak observed on Subnet-Calculator after the
|
||||
// Link-only version of this fix shipped).
|
||||
expect(
|
||||
/milestonesPage\s*\+=\s*1/.test(body),
|
||||
"expected a page-increment loop for existing milestones"
|
||||
).toBe(true);
|
||||
expect(
|
||||
/\.headers\.get\(\s*["']link["']\s*\)/.test(body) &&
|
||||
/rel="next"/.test(body),
|
||||
"the milestones pagination loop must check the Link header (rel=\"next\")"
|
||||
).toBe(true);
|
||||
expect(
|
||||
/\.headers\.get\(\s*["']x-total-count["']\s*\)/.test(body),
|
||||
"the milestones pagination loop must fall back to X-Total-Count when Link header is absent (Gitea /milestones only emits X-Total-Count)"
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test("newly-created milestone must be cached into existingMilestones", () => {
|
||||
// Defensive: if `milestones` ever contains a same-named entry
|
||||
// twice (unlikely but cheap to guard), we shouldn't POST it twice.
|
||||
expect(
|
||||
/existingMilestones\.add\(\s*milestone\.title\s*\)/.test(body),
|
||||
"after a successful create, the new milestone title must be added to existingMilestones"
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("label dedup on sync", () => {
|
||||
const body = extractFunctionBody(
|
||||
SOURCE,
|
||||
/export async function mirrorGitRepoLabelsToGitea\b/
|
||||
);
|
||||
|
||||
test("body contains the per-label create branch we expect to guard", () => {
|
||||
expect(
|
||||
body.includes("existingLabels"),
|
||||
"expected the existingLabels set used for dedup"
|
||||
).toBe(true);
|
||||
expect(
|
||||
body.match(
|
||||
/await httpPost\(\s*`\$\{config\.giteaConfig\.url\}\/api\/v1\/repos\/\$\{giteaOwner\}\/\$\{repoName\}\/labels`/
|
||||
),
|
||||
"expected the create-label httpPost call"
|
||||
).toBeTruthy();
|
||||
});
|
||||
|
||||
test("existing-labels GET must paginate with both Link and X-Total-Count fallback", () => {
|
||||
// Same Gitea MAX_RESPONSE_ITEMS=50 cap as milestones / issues.
|
||||
// Gitea's /labels endpoint, like /milestones, does NOT emit a Link
|
||||
// header — only `X-Total-Count`. Strict Link-only check would
|
||||
// silently truncate after page 1.
|
||||
expect(
|
||||
/labelsPage\s*\+=\s*1/.test(body),
|
||||
"expected a page-increment loop for existing labels"
|
||||
).toBe(true);
|
||||
expect(
|
||||
/\.headers\.get\(\s*["']link["']\s*\)/.test(body) &&
|
||||
/rel="next"/.test(body),
|
||||
"the labels pagination loop must check the Link header (rel=\"next\")"
|
||||
).toBe(true);
|
||||
expect(
|
||||
/\.headers\.get\(\s*["']x-total-count["']\s*\)/.test(body),
|
||||
"the labels pagination loop must fall back to X-Total-Count when Link header is absent (Gitea /labels only emits X-Total-Count)"
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
test("newly-created label must be cached into existingLabels", () => {
|
||||
expect(
|
||||
/existingLabels\.add\(\s*label\.name\s*\)/.test(body),
|
||||
"after a successful create, the new label name must be added to existingLabels"
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
+95
-20
@@ -3408,17 +3408,49 @@ export async function mirrorGitRepoLabelsToGitea({
|
||||
return;
|
||||
}
|
||||
|
||||
// Get existing labels from Gitea
|
||||
const giteaLabelsRes = await httpGet(
|
||||
`${config.giteaConfig.url}/api/v1/repos/${giteaOwner}/${repoName}/labels`,
|
||||
{
|
||||
Authorization: `token ${decryptedConfig.giteaConfig.token}`,
|
||||
}
|
||||
);
|
||||
// Get existing labels from Gitea. Paginate because Gitea caps
|
||||
// response size at `[api].MAX_RESPONSE_ITEMS` (default 50), so a
|
||||
// single unpaginated GET only sees the first 50 labels. Once a repo
|
||||
// crosses that threshold every label past it would be re-POSTed as a
|
||||
// duplicate on every sync.
|
||||
//
|
||||
// Pagination signal: prefer Link header (RFC 5988) when present, but
|
||||
// Gitea's /labels and /milestones endpoints do NOT emit Link headers
|
||||
// — they only emit `X-Total-Count`. Without the fallback, a strict
|
||||
// Link-only check terminated after page 1 and re-POSTed every label
|
||||
// past index 50 on every sync. (Repro found during the milestone
|
||||
// dedup fix: 9 unique milestones leaked past page 1 on a 74-row
|
||||
// /milestones response.)
|
||||
const existingLabels = new Set<string>();
|
||||
const labelsPerPage = 50;
|
||||
let labelsPage = 1;
|
||||
let labelsFetched = 0;
|
||||
while (true) {
|
||||
const giteaLabelsRes = await httpGet(
|
||||
`${config.giteaConfig.url}/api/v1/repos/${giteaOwner}/${repoName}/labels?page=${labelsPage}&limit=${labelsPerPage}`,
|
||||
{
|
||||
Authorization: `token ${decryptedConfig.giteaConfig.token}`,
|
||||
}
|
||||
);
|
||||
const pageLabels = Array.isArray(giteaLabelsRes.data) ? giteaLabelsRes.data : [];
|
||||
if (!pageLabels.length) break;
|
||||
for (const lbl of pageLabels) existingLabels.add(lbl.name);
|
||||
labelsFetched += pageLabels.length;
|
||||
|
||||
const existingLabels = new Set(
|
||||
giteaLabelsRes.data.map((label: any) => label.name)
|
||||
);
|
||||
const linkHeader = giteaLabelsRes.headers.get("link") || "";
|
||||
if (/\brel="next"/.test(linkHeader)) {
|
||||
labelsPage += 1;
|
||||
continue;
|
||||
}
|
||||
// No Link header (or no rel=next). Fall back to X-Total-Count.
|
||||
const totalStr = giteaLabelsRes.headers.get("x-total-count");
|
||||
const total = totalStr ? Number.parseInt(totalStr, 10) : NaN;
|
||||
if (Number.isFinite(total) && labelsFetched < total) {
|
||||
labelsPage += 1;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
let mirroredCount = 0;
|
||||
for (const label of labels) {
|
||||
@@ -3435,6 +3467,9 @@ export async function mirrorGitRepoLabelsToGitea({
|
||||
Authorization: `token ${decryptedConfig.giteaConfig.token}`,
|
||||
}
|
||||
);
|
||||
// Track locally so a duplicate in `labels` (shouldn't happen,
|
||||
// but defensive) doesn't trigger a second POST in the same run.
|
||||
existingLabels.add(label.name);
|
||||
mirroredCount++;
|
||||
} catch (error) {
|
||||
console.error(
|
||||
@@ -3508,17 +3543,54 @@ export async function mirrorGitRepoMilestonesToGitea({
|
||||
return;
|
||||
}
|
||||
|
||||
// Get existing milestones from Gitea
|
||||
const giteaMilestonesRes = await httpGet(
|
||||
`${config.giteaConfig.url}/api/v1/repos/${giteaOwner}/${repoName}/milestones`,
|
||||
{
|
||||
Authorization: `token ${decryptedConfig.giteaConfig.token}`,
|
||||
}
|
||||
);
|
||||
// Get existing milestones from Gitea. Two correctness requirements:
|
||||
// 1. `state=all` — Gitea's /milestones endpoint defaults to OPEN
|
||||
// only, so without this every CLOSED GitHub milestone is
|
||||
// misclassified as missing and re-POSTed on every sync. This
|
||||
// was the root cause of the 11k+ duplicate-closed-milestone
|
||||
// blowup observed in production.
|
||||
// 2. Pagination via Link header (RFC 5988) — Gitea caps response
|
||||
// size at `[api].MAX_RESPONSE_ITEMS` (default 50), so any repo
|
||||
// with more than ~50 milestones in a given state silently
|
||||
// truncates without it. Same Gitea-side cap that bit the
|
||||
// issues / PRs pre-fetch in commit b76073b.
|
||||
// Pagination signal: prefer Link header (RFC 5988) when present, but
|
||||
// Gitea's /milestones endpoint does NOT emit a Link header — it only
|
||||
// emits `X-Total-Count`. A strict Link-only check terminates after
|
||||
// page 1 and re-POSTs every milestone past index 50 on every sync.
|
||||
// (Repro: post-fix deploy on Subnet-Calculator leaked 9 unique
|
||||
// milestones past page 1 of a 74-row /milestones response.)
|
||||
const existingMilestones = new Set<string>();
|
||||
const milestonesPerPage = 50;
|
||||
let milestonesPage = 1;
|
||||
let milestonesFetched = 0;
|
||||
while (true) {
|
||||
const giteaMilestonesRes = await httpGet(
|
||||
`${config.giteaConfig.url}/api/v1/repos/${giteaOwner}/${repoName}/milestones?state=all&page=${milestonesPage}&limit=${milestonesPerPage}`,
|
||||
{
|
||||
Authorization: `token ${decryptedConfig.giteaConfig.token}`,
|
||||
}
|
||||
);
|
||||
const pageMilestones = Array.isArray(giteaMilestonesRes.data)
|
||||
? giteaMilestonesRes.data
|
||||
: [];
|
||||
if (!pageMilestones.length) break;
|
||||
for (const ms of pageMilestones) existingMilestones.add(ms.title);
|
||||
milestonesFetched += pageMilestones.length;
|
||||
|
||||
const existingMilestones = new Set(
|
||||
giteaMilestonesRes.data.map((milestone: any) => milestone.title)
|
||||
);
|
||||
const linkHeader = giteaMilestonesRes.headers.get("link") || "";
|
||||
if (/\brel="next"/.test(linkHeader)) {
|
||||
milestonesPage += 1;
|
||||
continue;
|
||||
}
|
||||
const totalStr = giteaMilestonesRes.headers.get("x-total-count");
|
||||
const total = totalStr ? Number.parseInt(totalStr, 10) : NaN;
|
||||
if (Number.isFinite(total) && milestonesFetched < total) {
|
||||
milestonesPage += 1;
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
let mirroredCount = 0;
|
||||
for (const milestone of milestones) {
|
||||
@@ -3536,6 +3608,9 @@ export async function mirrorGitRepoMilestonesToGitea({
|
||||
Authorization: `token ${decryptedConfig.giteaConfig.token}`,
|
||||
}
|
||||
);
|
||||
// Track locally so a duplicate within `milestones` (shouldn't
|
||||
// happen, but defensive) doesn't trigger a second POST.
|
||||
existingMilestones.add(milestone.title);
|
||||
mirroredCount++;
|
||||
} catch (error) {
|
||||
console.error(
|
||||
|
||||
Reference in New Issue
Block a user