diff --git a/tests/helpers/github-metadata-exchange.mjs b/tests/helpers/github-metadata-exchange.mjs index 8d65e3399f..8deb05e826 100644 --- a/tests/helpers/github-metadata-exchange.mjs +++ b/tests/helpers/github-metadata-exchange.mjs @@ -3,6 +3,7 @@ import { get } from "node:https"; // The test plays the enrolled supervisor. OCE owns the genuine signed installation // exchange, private response acceptance and material/dispatch/response decisions. +// `raceMaintenance` is the prepared GitHub owner race these tests share. export function captureMetadata(f, changed = {}) { const captured = f.capture(); const [route] = f.installation.plan.routes; @@ -75,3 +76,14 @@ export async function dispatchMetadata( } return protectedResponse; } + +/** Runs one maintenance pass on every owner at once over the same durable family. Exactly one + * owner claims the due family and commits; returns the other owners' results, sorted. A loser + * claims nothing, so it reports only its own retirement step: `idle`, or `retired` when a + * superseded token was already in retirement custody by then. */ +export async function raceMaintenance(owners, signal) { + const results = await Promise.all(owners.map((owner) => owner.maintain(signal))); + const committed = results.filter((result) => result === "committed"); + assert.equal(committed.length, 1, `exactly one owner must commit: ${results.join(", ")}`); + return results.filter((result) => result !== "committed").sort(); +} diff --git a/tests/integration/postgres-prepared-github-lifecycle.test.mjs b/tests/integration/postgres-prepared-github-lifecycle.test.mjs index 4fecf5cfd5..99941a4909 100644 --- a/tests/integration/postgres-prepared-github-lifecycle.test.mjs +++ b/tests/integration/postgres-prepared-github-lifecycle.test.mjs @@ -6,7 +6,11 @@ import { join } from "node:path"; import test from "node:test"; import pg from "pg"; import { setTimeout as delay } from "node:timers/promises"; -import { captureMetadata, dispatchMetadata } from "../helpers/github-metadata-exchange.mjs"; +import { + captureMetadata, + dispatchMetadata, + raceMaintenance, +} from "../helpers/github-metadata-exchange.mjs"; import { postgresEgressSendReceipts } from "../../packages/occ/src/state/egress-send-receipts.ts"; import { bindRuntimeSource } from "../helpers/postgres-protected-scenarios.mjs"; import { postgresApp } from "../helpers/postgres-egress-app.mjs"; @@ -53,9 +57,10 @@ test( // Two workers race over a real family. Only the durable claim holder reaches the issuer. const competitor = createOwner(); await competitor.connect(f.pool); - const results = await Promise.all([lifecycle.maintain(signal), competitor.maintain(signal)]); + // The family is cold, so the loser has nothing to retire. + const losers = await raceMaintenance([lifecycle, competitor], signal); await competitor.close(); - assert.deepEqual(results.sort(), ["committed", "idle"]); + assert.deepEqual(losers, ["idle"]); const old = await lifecycle.readWarm(source, readWindow()); assert.equal(old.ownerEpoch, 1); assert.deepEqual(gh.issuesOfTokens[0].permissions, { metadata: "read" }); diff --git a/tests/integration/postgres-prepared-token-service.test.ts b/tests/integration/postgres-prepared-token-service.test.ts index a08d9d9e84..d3ad5710e2 100644 --- a/tests/integration/postgres-prepared-token-service.test.ts +++ b/tests/integration/postgres-prepared-token-service.test.ts @@ -38,7 +38,11 @@ import { import { bindRuntimeSource } from "../helpers/postgres-protected-scenarios.mjs"; import { postgresApp } from "../helpers/postgres-egress-app.mjs"; import { requiresPostgres } from "../helpers/postgres-database.mjs"; -import { captureMetadata, dispatchMetadata } from "../helpers/github-metadata-exchange.mjs"; +import { + captureMetadata, + dispatchMetadata, + raceMaintenance, +} from "../helpers/github-metadata-exchange.mjs"; import { staticEgressFixture } from "../helpers/static-egress.mjs"; import { temporaryDirectory } from "../fixtures/repository-credentials/process.mjs"; import { @@ -632,11 +636,19 @@ test( // Advance only the polling schedule, not token validity. Separate owners must coalesce // against the same durable family even though the rejected token is still fresh. await due(); - const results = await Promise.all([ - restarted.maintain(t.signal), - competitor.maintain(t.signal), - ]); - assert.deepEqual(results.sort(), ["committed", "idle"]); + // Revocations of the rejected token: the test's own revocation of `late`, then retirement. + const [{ index: rejected }] = gh.issuesOfTokens; + const revocations = (index = rejected) => + gh.trace.filter( + ({ method, target, tokenIndex }) => + method === "DELETE" && target === "/installation/token" && tokenIndex === index, + ).length; + assert.equal(revocations(), 1); + // The commit moves the rejected token into retirement custody. A loser whose pass reaches + // its retirement step after that commit retires it (`retired`); otherwise the next pass does. + const [loser] = await raceMaintenance([restarted, competitor], t.signal); + assert(loser === "idle" || loser === "retired", `unexpected losing pass: ${loser}`); + assert.equal(revocations(), loser === "retired" ? 2 : 1); const replacement = await restarted.readWarm(source, window()); assert(replacement); assert.notEqual(replacement.materialVersion, old.materialVersion); @@ -646,6 +658,7 @@ test( assert.equal(await restartedReceipts.reject(late.sender, late, late.material), "fenced"); await due(); assert.equal(await restarted.maintain(t.signal), "ready"); + assert.equal(revocations(), 2, "exactly one pass retires the rejected token"); assert.equal( (await restarted.readWarm(source, window()))?.materialVersion, replacement.materialVersion, @@ -662,6 +675,7 @@ test( ).length, 2, ); + assert.equal(revocations(gh.issuesOfTokens[1]!.index), 0, "never retire the replacement"); const next = await dispatched(true); gh.disconnectAfterMutation("POST", `/app/installations/${fixtureInstallationId}/access_tokens`); await due(); @@ -672,6 +686,7 @@ test( assert.equal(await competitor.maintain(t.signal), "blocked"); assert.equal(gh.issuesOfTokens.length, 3, "rejection cannot replay uncertain issuance"); assert.equal(await competitor.readWarm(source, window()), undefined); + assert.equal(revocations(), 2, "later passes never replay the rejected token's retirement"); assert.deepEqual(gh.errors, []); }, );