diff --git a/.github/scripts/ci/main-failure-signature.mjs b/.github/scripts/ci/main-failure-signature.mjs index 495f55cd1e8..17f812f4fe1 100644 --- a/.github/scripts/ci/main-failure-signature.mjs +++ b/.github/scripts/ci/main-failure-signature.mjs @@ -42,7 +42,55 @@ const VITEST_FAIL_PATTERN = /^FAIL\s+(.+)$/; // Anchoring on ` - ` rather than the first space keeps parametrized node ids // whose parameters contain spaces (`test_x[case one]`). const PYTEST_FAIL_PATTERN = /^FAILED\s+(.+?)(?:\s+-\s.*)?$/; +// Maven Surefire prints one line per failed test as its class finishes: +// `[ERROR] com.example.FooTest.caseName -- Time elapsed: 0.569 s <<< ERROR!` +// (`<<< FAILURE!` for a failed assertion). The elapsed time varies run to +// run, so the identifier is the class-and-method id ahead of it. The +// class-level `Tests run: N ... <<< FAILURE! -- in ` line names no +// method and is not a test. +const MAVEN_FAIL_PATTERN = + /^\[ERROR\] (.+?) -- Time elapsed: [\d.]+ s <<< (?:ERROR|FAILURE)!$/; +// The Flyway version guard (scripts/check-flyway-migrations.js) fails with a +// ::error:: line, not a test report. Key it on the module and version — +// stable across every merge stacked on a standing red, so the recurrences +// land on one issue instead of one per commit. The runner consumes the +// ::error:: command and writes ##[error] into the downloadable log (stderr is +// wired to the same interceptor), so the anchor accepts both forms; the raw +// form is what the guard prints and keeps the unit fixtures readable. +const FLYWAY_FAIL_PATTERN = + /^(?:##\[error\]|::error::)(.+?): \d+ migrations claim version ([\d.]+):/; +// The guard's two other failure modes — a migration location that moved and +// a module directory that does not exist — are standing reds too, so they get +// their own stable identity keyed on the module and the mode; without one +// they fall back to the sha-keyed per-commit issue and each stacked merge +// opens a fresh one. The location half is tightened to its real charset (the +// guard emits its LOCATIONS constant joined with `/`): a runner-decoded +// annotation is the runner's own rendering, and an unbounded [^;]+ would let +// a filename-borne payload smuggle spaces and backticks into an id that is +// rendered inside a code span in the issue body. +const FLYWAY_CONFIG_PATTERN = + /^(?:##\[error\]|::error::)(.+?): (no such Maven module directory|found no migration under [\w./-]+|found migration files outside [\w./-]+)/; const TEST_FILE_PATTERN = /\.(?:test|spec)\.[cm]?[jt]sx?\b|\.py\b/; +// A Surefire id is a dotted `Class.method`, optionally with the parameter +// types and invocation index of a parameterized case. +const JAVA_ID_PATTERN = /^(?:[\w$]+\.)+[\w$]+(?:\([^)]*\))?(?:\[\d+])?$/; +// The module half of a guard id is untrusted: the line it is parsed from may +// be a runner-decoded continuation of a migration PATH (git carries LF, `:` +// and `\` in names, and a directory component puts the forged text after a +// real `/`). The timestamp gate in `extractFailingTests` rejects those +// continuations outright; this shape is the second layer for every other +// channel. The genuine value always comes from the invocation's repo-relative +// paths — the producer job and the analyze job both run on ubuntu-latest, and +// this suite's fixtures normalize their argument the same way — so it is +// slash-bearing with no whitespace or backslashes, and anything else, +// including a forged Windows drive prefix, is a payload. Backticks are +// rejected outright because the id is rendered inside a code span in the +// issue body. +const GUARD_MODULE_UNSAFE = /[\s`\\]/; + +function guardModule(raw) { + return !GUARD_MODULE_UNSAFE.test(raw) && raw.includes('/') ? raw : undefined; +} function cleanLine(line) { return line @@ -52,18 +100,80 @@ function cleanLine(line) { .trim(); } +/** + * The Surefire id is the dedupe identity — the `seen` key, the + * `qwen-main-ci-failure-test:` marker, the bullet the body lists — so it is + * kept whole: two modules can carry the same simple `Class.method` (sdk-java + * already has three such simple-name pairs), and reducing to `Class.method` + * would merge them into one issue. The JAVA_ID_PATTERN shape is this lane's + * own guard against the phrase appearing in a test's captured stdout. + * Cosmetic shortening for the title lives in `shortenForTitle`. + */ +function javaTestId(raw) { + return JAVA_ID_PATTERN.test(raw) ? raw : undefined; +} + /** * Collect the failing test identifiers a runner reported, first-seen order. - * Both runners print their failures more than once (inline plus summary), and a + * Every runner prints its failures more than once (inline plus summary), and a * matrix leg repeats them per job, so identifiers are deduped. */ export function extractFailingTests(logText) { const seen = new Set(); for (const rawLine of String(logText ?? '').split('\n')) { + // A flyway id is built from untrusted path text, so it is accepted only + // from a line the runner itself emitted — one carrying its timestamp. + // The guard prints file PATHS, so a LF inside a directory component + // leaves the forged text after a real `/` and past the module shape + // below; but a decoded annotation continuation is written without the + // runner's timestamp, which closes that door for both flyway arms. + const runnerLine = LOG_TIMESTAMP_PATTERN.test( + rawLine.replace(ANSI_PATTERN, ''), + ); const line = cleanLine(rawLine); const vitest = VITEST_FAIL_PATTERN.exec(line); const pytest = PYTEST_FAIL_PATTERN.exec(line); - if (!vitest && !pytest) continue; + const maven = MAVEN_FAIL_PATTERN.exec(line); + const flyway = FLYWAY_FAIL_PATTERN.exec(line); + const flywayConfig = FLYWAY_CONFIG_PATTERN.exec(line); + if (!vitest && !pytest && !maven && !flyway && !flywayConfig) continue; + + // The flyway id doubles as the issue title and body bullet, so it is + // written to be read, not only to be hashed — and both captures are + // validated before they become a dedupe identity, the way the Surefire + // id is validated below. + if (flyway) { + if (!runnerLine) continue; + const module = guardModule(flyway[1]); + if (module) { + seen.add(`flyway duplicate version ${flyway[2]} in ${module}`); + continue; + } + // A rejected capture must not discard a genuine config-mode identity + // on the same line — fall through to the flywayConfig arm below + // instead of returning. The suppression payload needs no newline at + // all: spaces on the timestamped line run the lazy module capture all + // the way to the payload, and an unconditional continue here would + // drop the line's real diagnosis with it. + } + + if (flywayConfig) { + if (!runnerLine) continue; + const module = guardModule(flywayConfig[1]); + if (module) seen.add(`flyway ${flywayConfig[2]} in ${module}`); + continue; + } + + // A line either flyway pattern matched is fully handled above, whether + // it yielded an identity or was dropped — the shared tail below reads + // vitest/pytest captures unconditionally and is only safe past it. + if (flyway || flywayConfig) continue; + + if (maven) { + const id = javaTestId(maven[1]); + if (id) seen.add(id); + continue; + } // pytest -q appends ` - `; the message varies run to run and // would defeat deduping, so keep only the `file::test` node id. @@ -122,7 +232,15 @@ export function failureSignature(workflowName, testIds) { * them (`file > Suite > nested > case` is routinely over 140 characters). */ export function shortenForTitle(testId, limit = 110) { - const segments = testId.replace(/\s+/g, ' ').trim().split(' > '); + const text = testId.replace(/\s+/g, ' ').trim(); + // A Surefire id's package prefix adds no signal next to the class name and + // pushes the method out of the title budget, and package segments are + // lowercase by convention. Only a Java-shaped id is stripped — a vitest + // `file > case` id opens with a path that must survive intact. + const display = JAVA_ID_PATTERN.test(text) + ? text.replace(/^(?:[a-z_$][\w$]*\.)+/, '') + : text; + const segments = display.split(' > '); const collapsed = segments.length > 2 ? [segments[0], '…', segments.at(-1)].join(' > ') @@ -141,6 +259,18 @@ export function analyzeLogs(workflowName, logTexts, failedJobs = []) { } } + // Guard-derived ids are the only run-level diagnoses in the set: the + // collision they name is stable across every merge stacked on a standing + // red, so they title the issue and are searched first. First-seen order + // would let a mass failure in another lane crowd them past the search and + // body caps — nothing guarantees the guard's log is globbed first. The + // sort is stable, so each group keeps its first-seen order and the cap + // counts are unchanged. + tests.sort( + (a, b) => + Number(b.id.startsWith('flyway ')) - Number(a.id.startsWith('flyway ')), + ); + const extra = tests.length > 1 ? ` (+${tests.length - 1} more)` : ''; return { workflow: workflowName, @@ -175,6 +305,12 @@ const ALSO_FAILING_HEADING = '## Also failing'; // is the heading plus its contiguous bullet list — nothing else is ever written // under it. const ALSO_FAILING_BLOCK = /\n*##\s+Also failing\s*\n+(?:- [^\n]*\n?)+/; +// The "- Failed jobs:" block is machine-owned the same way: on merge it is +// stripped and re-emitted from the CURRENT run's failed jobs, so the lane the +// body points at never freezes at filing time. The bullets it owns are the +// two-space ones failedJobLines emits — the strip stops at the first line +// that is not one, never consuming a human bullet that starts with `- `. +const FAILED_JOBS_BLOCK = /\n*- Failed jobs:\n(?: {2}- [^\n]*\n?)+/; // The same split/merge contract — head / recorded occurrences / tail around // the marker, human text kept verbatim, occurrences newest-first and capped — @@ -284,6 +420,12 @@ export function renderIssueBody({ ...bodyMarkers.map((marker) => ``), '', `A main-branch \`${analysis.workflow}\` run failed on \`main\`.`, + // The failing tests name what broke; the failed jobs name which lane + // broke — a guard's diagnosis (the colliding files) lives only in its + // job log. + ...(analysis.failedJobs.length + ? ['', '- Failed jobs:', ...failedJobLines(analysis.failedJobs)] + : []), '', '## Failing tests', '', @@ -312,8 +454,13 @@ export function renderIssueBody({ // The "## Also failing" list is rebuilt from the current failure set below, // so strip the previous one first: a test that has since been fixed must - // disappear instead of being listed forever. - const strippedProse = prose.replace(ALSO_FAILING_BLOCK, '').trimEnd(); + // disappear instead of being listed forever. The "- Failed jobs:" block is + // rebuilt from the current run the same way; the '\n' replacement keeps the + // blank line that separated a mid-prose block from what followed it. + const strippedProse = prose + .replace(ALSO_FAILING_BLOCK, '') + .replace(FAILED_JOBS_BLOCK, '\n') + .trimEnd(); // Record markers for tests that joined the failure set after the issue was // opened, so the next run still matches this issue on either test. @@ -326,9 +473,14 @@ export function renderIssueBody({ const withMarkers = missingMarkers.length ? `${missingMarkers.map((marker) => ``).join('\n')}\n${strippedProse}` : strippedProse; - const withTests = missingTests.length - ? `${withMarkers}\n\n${ALSO_FAILING_HEADING}\n\n${missingTests.join('\n')}` + // A run whose failed-job list came back empty drops the block entirely + // rather than freezing the lanes an earlier run happened to report. + const withJobs = analysis.failedJobs.length + ? `${withMarkers}\n\n- Failed jobs:\n${failedJobLines(analysis.failedJobs).join('\n')}` : withMarkers; + const withTests = missingTests.length + ? `${withJobs}\n\n${ALSO_FAILING_HEADING}\n\n${missingTests.join('\n')}` + : withJobs; // A re-run of the same run must not add a second line for it. Match the // `[run ]` link text, not the run URL: `/301` is a substring of `/3010`, diff --git a/.github/scripts/ci/main-failure-signature.test.mjs b/.github/scripts/ci/main-failure-signature.test.mjs index a55945ccba4..52c55806edc 100644 --- a/.github/scripts/ci/main-failure-signature.test.mjs +++ b/.github/scripts/ci/main-failure-signature.test.mjs @@ -1,8 +1,17 @@ import assert from 'node:assert/strict'; -import { mkdtempSync, writeFileSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join, sep } from 'node:path'; import test from 'node:test'; +import { fileURLToPath } from 'node:url'; import { LEGACY_MARKER_PREFIX, @@ -60,6 +69,483 @@ test('keeps the full pytest node id when parameters contain spaces', () => { ]); }); +// Verbatim lines of SDK Java run 36382763760 (the duplicate-V16 merge of +// #12940): one inline Surefire line per errored test, each preceded by the +// class-level summary line that names no method and must not register. +const MAVEN_LOG = [ + '2026-09-28T05:40:26.0787691Z [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.592 s <<< FAILURE! -- in com.alibaba.qwen.code.managedagent.ManagedSessionOperationMigrationTest', + '2026-09-28T05:40:26.0790051Z [ERROR] com.alibaba.qwen.code.managedagent.ManagedSessionOperationMigrationTest.finishesLifecycleCommandsThatWaitedBeforeTheUpgrade -- Time elapsed: 0.569 s <<< ERROR!', + '2026-09-28T05:40:26.2087318Z [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.060 s <<< FAILURE! -- in com.alibaba.qwen.code.managedagent.ManagedEventIdentityMigrationTest', + '2026-09-28T05:40:26.2115597Z [ERROR] com.alibaba.qwen.code.managedagent.ManagedEventIdentityMigrationTest.backfillsTheIdentityThatTheSnapshotUses -- Time elapsed: 0.058 s <<< ERROR!', +].join('\n'); + +test('extracts surefire failures without the elapsed time', () => { + assert.deepEqual(extractFailingTests(MAVEN_LOG), [ + 'com.alibaba.qwen.code.managedagent.ManagedSessionOperationMigrationTest.finishesLifecycleCommandsThatWaitedBeforeTheUpgrade', + 'com.alibaba.qwen.code.managedagent.ManagedEventIdentityMigrationTest.backfillsTheIdentityThatTheSnapshotUses', + ]); +}); + +test('extracts a surefire assertion failure, deduped across matrix legs', () => { + const log = [ + '[ERROR] com.example.LedgerTest.totals -- Time elapsed: 0.011 s <<< FAILURE!', + '[ERROR] com.example.LedgerTest.totals -- Time elapsed: 0.009 s <<< FAILURE!', + ].join('\n'); + assert.deepEqual(extractFailingTests(log), ['com.example.LedgerTest.totals']); +}); + +test('keeps two same-named tests from different packages distinct', () => { + // The Surefire id is the dedupe identity, so the package prefix must + // survive: sdk-java already carries the same simple test class name in two + // Maven modules, and reducing to `Class.method` would merge them into one + // issue. + const log = [ + '[ERROR] com.a.SessionTest.closes -- Time elapsed: 0.10 s <<< ERROR!', + '[ERROR] com.b.SessionTest.closes -- Time elapsed: 0.11 s <<< ERROR!', + ].join('\n'); + assert.deepEqual(extractFailingTests(log), [ + 'com.a.SessionTest.closes', + 'com.b.SessionTest.closes', + ]); + const analysis = analyzeLogs('SDK Java', [log]); + assert.equal(analysis.markers.length, 2); + assert.notEqual(analysis.markers[0], analysis.markers[1]); +}); + +test('ignores a Maven error line whose payload is not a Java id', () => { + assert.deepEqual( + extractFailingTests( + '[ERROR] model refused the request -- Time elapsed: 1.2 s <<< ERROR!', + ), + [], + ); +}); + +test('keeps the parameterized tail of a Surefire id', () => { + assert.deepEqual( + extractFailingTests( + '[ERROR] com.example.FooTest.bar(java.util.List)[1] -- Time elapsed: 0.10 s <<< ERROR!', + ), + ['com.example.FooTest.bar(java.util.List)[1]'], + ); +}); + +const FLYWAY_GUARD = join( + dirname(fileURLToPath(import.meta.url)), + '../../../scripts/check-flyway-migrations.js', +); + +// The producer job and the analyze job both run on ubuntu-latest, so the +// module half of a guard id is always slash-bearing — and the consumer's +// validator rejects backslashes outright, because a POSIX filename CAN hold +// them (a forged `C:\evil` prefix is a filename away). Hand the guard a +// slash-bearing argument too (a no-op on POSIX; Node accepts forward slashes +// on Windows) so a local Windows run of this suite exercises the same shape +// instead of teaching the consumer a shape that only a forgery can produce. +const slashPath = (dir) => dir.split(sep).join('/'); + +// The Flyway fixtures are CAPTURED from the guard, never typed by hand: a +// drift between the producer's message and this consumer's pattern must +// redden here — silently surviving a reworded producer is the failure this +// module exists to end. The captured stderr is then rendered the way a +// downloaded Actions log carries it: the runner consumes the ::error:: +// command, writes ##[error] in its place, and timestamps every line. +function captureGuardLog(moduleDir) { + const result = spawnSync(process.execPath, [FLYWAY_GUARD, moduleDir], { + encoding: 'utf8', + }); + assert.equal(result.status, 1, `guard stderr: ${result.stderr}`); + const raw = result.stderr + .split('\n') + .filter((line) => line.startsWith('::error::')) + .map((line) => `2026-09-29T00:00:00.0000000Z ${line}`) + .join('\n'); + return { raw, tagged: raw.replaceAll('::error::', '##[error]') }; +} + +function planSearchMarkers(log, sha) { + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-plan-')); + const analysisPath = join(dir, 'analysis.json'); + writeFileSync(analysisPath, JSON.stringify(analyzeLogs('SDK Java', [log]))); + return JSON.parse( + captureStdout([ + 'plan', + '--analysis', + analysisPath, + '--sha', + sha, + '--run-url', + `https://github.com/QwenLM/qwen-code/actions/runs/${sha}`, + '--run-id', + sha, + '--at', + '2026-09-29T00:00:00Z', + ]), + ).searchMarkers; +} + +test('a Flyway guard red files one issue, not one per stacked merge', () => { + // Two SQL files claiming one version in one module: the #12940 shape. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + mkdirSync(migrationDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V16__a.sql'), ''); + writeFileSync(join(migrationDir, 'V16__b.sql'), ''); + + const log = captureGuardLog(moduleDir); + const id = `flyway duplicate version 16 in ${moduleDir}`; + // The ##[error] form is what a downloaded job log carries; the raw + // ::error:: form is what the guard prints. Both must extract. + assert.deepEqual(extractFailingTests(log.tagged), [id]); + assert.deepEqual(extractFailingTests(log.raw), [id]); + + // The marker keys on the collision, not the commit: two merges stacked on + // the standing red resolve to the same issue. + assert.deepEqual( + planSearchMarkers(log.tagged, '1111111111111'), + planSearchMarkers(log.tagged, '2222222222222'), + ); +}); + +test('a Flyway guard blind-spot red also files one issue, not one per stacked merge', () => { + // A renamed migration location and a missing module directory are standing + // reds that name no test: without their own stable identity the plan falls + // back to the sha-keyed per-commit marker and each stacked merge opens a + // fresh issue. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-moved-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + // A db/migration* sibling marks the module as owning migrations while the + // configured SQL location stays empty — the guard's "location moved" mode. + mkdirSync(join(moduleDir, 'src/main/resources/db/migrations'), { + recursive: true, + }); + + const moved = captureGuardLog(moduleDir); + assert.deepEqual(extractFailingTests(moved.tagged), [ + `flyway found no migration under src/main/resources/db/migration in ${moduleDir}`, + ]); + assert.deepEqual( + planSearchMarkers(moved.tagged, '1111111111111'), + planSearchMarkers(moved.tagged, '2222222222222'), + ); + + const absentDir = slashPath(join(dir, 'absent-module')); + const absent = captureGuardLog(absentDir); + assert.deepEqual(extractFailingTests(absent.tagged), [ + `flyway no such Maven module directory in ${absentDir}`, + ]); + assert.deepEqual( + planSearchMarkers(absent.tagged, '1111111111111'), + planSearchMarkers(absent.tagged, '2222222222222'), + ); +}); + +test('a stray migration file outside a populated location gets a stable identity', () => { + // The message must not claim the location moved (it is populated) and the + // id must stay stable across stacked merges, so the standing red files + // one issue rather than one per commit. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-stray-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + mkdirSync(migrationDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V1__a.sql'), ''); + const scratch = join(moduleDir, 'src/main/resources/scratch'); + mkdirSync(scratch, { recursive: true }); + writeFileSync(join(scratch, 'V2__b.sql'), ''); + + const log = captureGuardLog(moduleDir); + const id = `flyway found migration files outside src/main/resources/db/migration in ${moduleDir}`; + assert.deepEqual(extractFailingTests(log.tagged), [id]); + assert.deepEqual(extractFailingTests(log.raw), [id]); + assert.deepEqual( + planSearchMarkers(log.tagged, '1111111111111'), + planSearchMarkers(log.tagged, '2222222222222'), + ); +}); + +// Filenames with an LF and `:` are un-creatable on the Windows lane, so gate +// the forgery fixtures on the CAPABILITY, not the platform — the probe +// scripts/tests/check-flyway-migrations.test.js sets the precedent for. +const newlineNamesWork = (() => { + const probe = mkdtempSync(join(tmpdir(), 'sig-name-probe-')); + try { + writeFileSync(join(probe, 'a\nb:c'), ''); + return true; + } catch { + return false; + } finally { + rmSync(probe, { recursive: true, force: true }); + } +})(); + +test( + 'a filename-borne forgery cannot inject a second flyway identity', + { skip: !newlineNamesWork && 'this filesystem cannot hold LF in names' }, + () => { + // The guard bounds the emitted charset, so nothing outside a migration + // path's real alphabet reaches the command data — the runner's decode + // finds no control byte to restore, and no continuation line can form. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-forge-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + mkdirSync(migrationDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V16__a.sql'), ''); + writeFileSync( + join( + migrationDir, + 'V16__x\n::error::evilmod: 2 migrations claim version 99: forged.sql', + ), + '', + ); + const log = captureGuardLog(moduleDir); + const id = `flyway duplicate version 16 in ${moduleDir}`; + assert.deepEqual(extractFailingTests(log.tagged), [id]); + + // Render the way the runner does: the command is consumed (its raw form + // is never echoed) and the annotation takes its place, the message + // percent-decoded — %0D/%0A first, %25 LAST, or the sequences the + // escaper emitted would decode twice. + const decoded = log.raw + .replace('::error::', '') + .replace(/%0D/g, '\r') + .replace(/%0A/g, '\n') + .replace(/%25/g, '%') + .replace('Z ', 'Z ##[error]'); + // The payload survives only as inert %XX text — no continuation, and + // the genuine identity still titles the collision. + assert.ok(!decoded.includes('\n::error::')); + assert.ok(!decoded.includes('\n[ERROR]')); + assert.deepEqual(extractFailingTests(decoded), [id]); + }, +); + +test( + 'a filename-borne Windows-drive forgery cannot inject a second flyway identity', + { skip: !newlineNamesWork && 'this filesystem cannot hold LF in names' }, + () => { + // The drive-prefix shape is a filename away (`\` and `:` are ordinary + // POSIX filename bytes); the emitted bound mangles both, so the decoded + // view carries neither the shape nor a continuation. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-drive-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + mkdirSync(migrationDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V16__a.sql'), ''); + writeFileSync( + join( + migrationDir, + 'V16__x\n::error::C:\\evil: 2 migrations claim version 99: forged.sql', + ), + '', + ); + const log = captureGuardLog(moduleDir); + const id = `flyway duplicate version 16 in ${moduleDir}`; + const decoded = log.raw + .replace('::error::', '') + .replace(/%0D/g, '\r') + .replace(/%0A/g, '\n') + .replace(/%25/g, '%') + .replace('Z ', 'Z ##[error]'); + assert.ok(!decoded.includes('\n::error::')); + assert.ok(!decoded.includes('C:\\evil')); + assert.deepEqual(extractFailingTests(decoded), [id]); + }, +); + +test('the module-shape validator rejects whitespace, backticks and backslashes', () => { + // A path-shaped capture (passes the `/` arm) that only the unsafe + // character half rejects — pinned for both characters and both call + // sites, because the rendered id sits inside a code span in the issue + // body: an odd backtick count leaves the span open. The lines carry the + // runner's timestamp so they reach the shape check past the gate. + const at = '2026-09-29T00:00:00.0000000Z '; + assert.deepEqual( + extractFailingTests( + `${at}##[error]evil/mod\`x: 2 migrations claim version 99: forged.sql`, + ), + [], + ); + assert.deepEqual( + extractFailingTests( + `${at}##[error]evil/mod x: 2 migrations claim version 99: forged.sql`, + ), + [], + ); + assert.deepEqual( + extractFailingTests( + `${at}##[error]evil/mod\`x: no such Maven module directory`, + ), + [], + ); + assert.deepEqual( + extractFailingTests( + `${at}##[error]evil\\mod/x: 2 migrations claim version 99: forged.sql`, + ), + [], + ); +}); + +test( + 'a directory-borne forgery cannot inject a slash-bearing flyway identity', + { skip: !newlineNamesWork && 'this filesystem cannot hold LF in names' }, + () => { + // A LF inside a DIRECTORY component would leave the forged text after a + // real `/` — slash-bearing — but the emitted bound mangles the LF and + // every pattern byte after it, so the decoded view forms no line. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-dirforge-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + const payloadDir = join( + migrationDir, + 'sub\n::error::packages/evil: 2 migrations claim version 99: y.sql', + ); + mkdirSync(payloadDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V16__a.sql'), ''); + writeFileSync(join(payloadDir, 'V16__b.sql'), ''); + const log = captureGuardLog(moduleDir); + const id = `flyway duplicate version 16 in ${moduleDir}`; + const decoded = log.raw + .replace('::error::', '') + .replace(/%0D/g, '\r') + .replace(/%0A/g, '\n') + .replace(/%25/g, '%') + .replace('Z ', 'Z ##[error]'); + assert.ok(!decoded.includes('\n::error::')); + assert.ok(!decoded.includes('packages/evil: 2 migrations claim')); + assert.deepEqual(extractFailingTests(decoded), [id]); + }, +); + +test( + 'a stray file carrying a forgery in a directory component yields only the genuine identity', + { skip: !newlineNamesWork && 'this filesystem cannot hold LF in names' }, + () => { + // The family's third entrance, planted where the stray diagnosis lives: + // a populated location plus a stray whose directory name carries the + // payload. At the unfixed head this extracts the forged id through the + // continuation AND drops the genuine config identity through the + // suppressed same-line capture; both arms must close. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-strayforge-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + mkdirSync(migrationDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V1__a.sql'), ''); + const payloadDir = join( + moduleDir, + 'src/main/resources/scratch', + 'x\n::error::pkgs', + ); + mkdirSync(payloadDir, { recursive: true }); + writeFileSync( + join(payloadDir, 'V2__: 2 migrations claim version 99: forged.sql'), + '', + ); + const log = captureGuardLog(moduleDir); + const id = `flyway found migration files outside src/main/resources/db/migration in ${moduleDir}`; + const decoded = log.raw + .replace('::error::', '') + .replace(/%0D/g, '\r') + .replace(/%0A/g, '\n') + .replace(/%25/g, '%') + .replace('Z ', 'Z ##[error]'); + assert.ok(!decoded.includes('\n::error::')); + assert.ok(!decoded.includes('claim version 99')); + assert.deepEqual(extractFailingTests(decoded), [id]); + }, +); + +test('a rejected duplicate-version capture must not discard the genuine config identity', () => { + // The suppression arm needs no newline and no runner behaviour: spaces on + // the timestamped line run the lazy duplicate-version capture all the way + // to the payload, and an unconditional continue there used to drop the + // line's genuine config-mode diagnosis with it — the duplicate storm of + // #12940 from the opposite direction. + const line = + '2026-09-29T00:00:00.0000000Z ##[error]packages/sdk-java/x: ' + + 'no such Maven module directory : 2 migrations claim version 99: '; + assert.deepEqual(extractFailingTests(line), [ + 'flyway no such Maven module directory in packages/sdk-java/x', + ]); +}); + +test('a guard diagnosis titles the issue and is searched even when its log sorts last', () => { + // Job ids — and therefore the failed-logs glob order — do not follow the + // workflow's declaration order, so the guard's log can arrive after a mass + // Surefire failure. First-seen order would crowd the one stable identity + // past the search cap; the run-level diagnosis must come first instead. + const dir = mkdtempSync(join(tmpdir(), 'sig-flyway-prio-')); + const moduleDir = slashPath(join(dir, 'managed-agent-server')); + const migrationDir = join(moduleDir, 'src/main/resources/db/migration'); + mkdirSync(migrationDir, { recursive: true }); + writeFileSync(join(migrationDir, 'V16__a.sql'), ''); + writeFileSync(join(migrationDir, 'V16__b.sql'), ''); + const flywayLog = captureGuardLog(moduleDir).tagged; + const mavenLog = Array.from( + { length: 8 }, + (_unused, index) => + `[ERROR] com.example.T${index}Test.case${index} -- Time elapsed: 0.001 s <<< FAILURE!`, + ).join('\n'); + + const analysis = analyzeLogs('SDK Java', [mavenLog, flywayLog]); + const id = `flyway duplicate version 16 in ${moduleDir}`; + assert.equal(analysis.tests[0].id, id); + assert.ok( + analysis.searchMarkers.includes(`${TEST_MARKER_PREFIX}${testKey(id)}`), + ); + // Priority is ordering, not extra markers: the cap is unchanged. + assert.equal(analysis.searchMarkers.length, MAX_SEARCH_MARKERS); +}); + +test('the spawned guard stays dependency-free for the pre-install lane', () => { + // This suite runs in HELPER_TESTS_DEP_FREE — before setup-node and any + // dependency install — and captureGuardLog spawns the guard, so the + // guard's WHOLE relative-import closure must stay node-builtins only. + // Walk it: every relative specifier is resolved and itself scanned, so a + // new hop carrying an npm import cannot slip past a depth-one list. + const queue = [FLYWAY_GUARD]; + const seen = new Set(); + for (let cursor = 0; cursor < queue.length; cursor += 1) { + const file = queue[cursor]; + if (seen.has(file)) continue; + seen.add(file); + for (const [, spec] of readFileSync(file, 'utf8').matchAll( + /from '([^']+)'/g, + )) { + if (spec.startsWith('node:')) continue; + assert.ok( + spec.startsWith('.'), + `${file} imports ${spec}; the dep-free lane spawns it before any install`, + ); + const resolved = join(dirname(file), spec); + assert.ok( + existsSync(resolved), + `${file} imports ${spec}, which does not resolve`, + ); + queue.push(resolved); + } + } +}); + +test('ignores Maven error lines that name no test', () => { + // Also verbatim from run 36382763760: the goal failure, the run-level + // tally, and the help pointer carry no `Class.method` failure line. + const log = [ + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.6:test (default-test) on project qwen-managed-agent-server:', + '[ERROR] Tests run: 153, Failures: 0, Errors: 88, Skipped: 0', + '[ERROR] -> [Help 1]', + ].join('\n'); + assert.deepEqual(extractFailingTests(log), []); +}); + +test('an SDK Java analysis titles the issue by the failing test', () => { + const analysis = analyzeLogs('SDK Java', [MAVEN_LOG]); + assert.equal( + analysis.title, + 'Main CI failed: SDK Java — ManagedSessionOperationMigrationTest.finishesLifecycleCommandsThatWaitedBeforeTheUpgrade (+1 more)', + ); +}); + test('keeps first-seen order across several failures', () => { const log = [ ' FAIL cli/b.test.ts > second', @@ -581,6 +1067,96 @@ test('the per-commit body names the failing job and step', () => { ); }); +test('the test-keyed body names the failed job and step too', () => { + // A red that names its tests still needs the lane: a guard's diagnosis (the + // colliding files) lives only in the job log, and the per-test bullets + // alone do not say which job broke. + const analysis = analyzeLogs( + 'SDK Java', + [MAVEN_LOG], + [ + { + name: 'Flyway migration version uniqueness', + steps: ['Check Flyway migration versions are unique'], + }, + ], + ); + const body = renderIssueBody({ analysis, occurrence: OCCURRENCE }); + + assert.ok(body.includes('## Failing tests')); + assert.ok( + body.includes( + ' - `Flyway migration version uniqueness` — failed in step `Check Flyway migration versions are unique`', + ), + ); +}); + +test('a test-keyed body with no reported jobs has no dangling heading', () => { + // The failed-jobs conditional's false branch: an unconditional spread + // would leave a "- Failed jobs:" heading with no bullets under it. + const analysis = analyzeLogs('SDK Java', [MAVEN_LOG]); + assert.deepEqual(analysis.failedJobs, []); + const body = renderIssueBody({ analysis, occurrence: OCCURRENCE }); + assert.ok(body.includes('## Failing tests')); + assert.ok(!body.includes('- Failed jobs:')); +}); + +test('merge rebuilds the failed-jobs block from the current run', () => { + // The block is machine-owned like "## Also failing": a later recurrence of + // the same failure can break in a different lane, and the body must point + // at the lane whose log holds THAT run's diagnosis. + const flywayLane = { + name: 'Flyway migration version uniqueness', + steps: ['Check Flyway migration versions are unique'], + }; + const databaseLane = { + name: 'Runtime Broker and Managed Agent MariaDB / Java 21', + steps: ['Run Managed Agent tests, Checkstyle, and MySQL integration'], + }; + const created = renderIssueBody({ + analysis: analyzeLogs('SDK Java', [MAVEN_LOG], [flywayLane]), + occurrence: OCCURRENCE, + }); + assert.ok(created.includes('`Flyway migration version uniqueness`')); + + const merged = renderIssueBody({ + analysis: analyzeLogs('SDK Java', [MAVEN_LOG], [databaseLane]), + existingBody: created, + occurrence: { ...OCCURRENCE, runId: '302', runUrl: '.../runs/302' }, + }); + assert.ok( + merged.includes( + ' - `Runtime Broker and Managed Agent MariaDB / Java 21` — failed in step `Run Managed Agent tests, Checkstyle, and MySQL integration`', + ), + ); + assert.ok(!merged.includes('Flyway migration version uniqueness')); + assert.equal(merged.split('- Failed jobs:').length - 1, 1); + + // A recurrence whose failed-job list is unknown drops the block rather + // than freezing a stale lane name. + const unknown = renderIssueBody({ + analysis: analyzeLogs('SDK Java', [MAVEN_LOG]), + existingBody: merged, + occurrence: { ...OCCURRENCE, runId: '303', runUrl: '.../runs/303' }, + }); + assert.ok(!unknown.includes('- Failed jobs:')); + + // And a run filed without the data gains the block once a recurrence + // reports it. + const gained = renderIssueBody({ + analysis: analyzeLogs('SDK Java', [MAVEN_LOG], [databaseLane]), + existingBody: renderIssueBody({ + analysis: analyzeLogs('SDK Java', [MAVEN_LOG]), + occurrence: OCCURRENCE, + }), + occurrence: { ...OCCURRENCE, runId: '304', runUrl: '.../runs/304' }, + }); + assert.ok(gained.includes('- Failed jobs:')); + assert.ok( + gained.includes('`Runtime Broker and Managed Agent MariaDB / Java 21`'), + ); +}); + test('a job with several failed steps lists every one of them', () => { const analysis = analyzeLogs( 'Qwen Code CI', diff --git a/.github/workflows/.size-baseline b/.github/workflows/.size-baseline index ad010b634c8..dbc592789be 100644 --- a/.github/workflows/.size-baseline +++ b/.github/workflows/.size-baseline @@ -30,7 +30,7 @@ 11634 finalize-release.yml 16647 live-host-release.yml 5950 live-host.yml -7642 main-ci-failure-issue.yml +11069 main-ci-failure-issue.yml 1343 mobile-shell.yml 2277 pnpm-store.yml 2489 pnpm-worktree-smoke.yml @@ -55,7 +55,7 @@ 35955 release.yml 44285 repo-hygiene.yml 1079 scorecard-monthly.yml -20678 sdk-java.yml +23524 sdk-java.yml 3886 sdk-python.yml 5595 security-checks.yml 6777 serve-ab-publish.yml diff --git a/.github/workflows/main-ci-failure-issue.yml b/.github/workflows/main-ci-failure-issue.yml index f2e4ac7bed3..fa51da72e87 100644 --- a/.github/workflows/main-ci-failure-issue.yml +++ b/.github/workflows/main-ci-failure-issue.yml @@ -13,7 +13,16 @@ on: # to surface a regression as it lands; a failure there files an issue with # `type/bug`, `status/ready-for-agent` and `autofix/approved`, so expect # one per red merge, not per red nightly only. - workflows: ['E2E Tests', 'SDK Python', 'Qwen Code CI'] + # 'SDK Java' is on the list for its post-merge push run. Its path filter + # watches the SDK's embedding surfaces too (packages/cli/src/serve/**, + # packages/acp-bridge/**, packages/cli/src/acp-integration/**), so it + # fires on roughly half of recent merges to `main`, not only on SDK + # changes — and that run is the only signal when a merge result is broken + # in a way neither PR could see: two green branches whose Flyway + # migrations claim one version collide only after both land (#12940: main + # red for two hours while three more merges stacked on it). It has no + # scheduled run, so the push arm covers it. + workflows: ['E2E Tests', 'SDK Java', 'SDK Python', 'Qwen Code CI'] types: ['completed'] # Trigger-level branch filter, not only the `if` below: 'Qwen Code CI' # completes on every pull request, and without this each of those would diff --git a/.github/workflows/sdk-java.yml b/.github/workflows/sdk-java.yml index 8490cf66f95..7c173946317 100644 --- a/.github/workflows/sdk-java.yml +++ b/.github/workflows/sdk-java.yml @@ -28,6 +28,7 @@ on: - 'scripts/run-java-daemon-sdk-e2e.ts' - 'scripts/run-managed-agent-server-e2e.ts' - 'scripts/check-failsafe-reports.js' + - 'scripts/check-flyway-migrations.js' - '.github/workflows/sdk-java.yml' - '.github/workflows/release-sdk-java.yml' push: @@ -57,6 +58,7 @@ on: - 'scripts/run-java-daemon-sdk-e2e.ts' - 'scripts/run-managed-agent-server-e2e.ts' - 'scripts/check-failsafe-reports.js' + - 'scripts/check-flyway-migrations.js' - '.github/workflows/sdk-java.yml' - '.github/workflows/release-sdk-java.yml' @@ -204,6 +206,28 @@ jobs: MAVEN_ARGS: '--settings ${{ runner.temp }}/setup-java-m2/settings.xml --toolchains ${{ runner.temp }}/setup-java-m2/toolchains.xml' run: 'mvn --batch-mode --no-transfer-progress -DskipTests -Dgpg.skip=true package' + # The Flyway duplicate-version guard of #12940. managed-agent-server + # allocates migration versions by hand across two locations that share one + # sequence, so two green PRs can collide once both merge; the test that + # catches it boots a database and runs only in the slow lanes below. This + # job is the fast lane: a filename scan, seconds in, on every pull request — + # whose checkout is the prospective merge result — and on every push. + # runtime-broker and qwencode carry no migrations today but share the + # namespace: managed-agent-server depends on both at compile scope, so + # Flyway resolves classpath:db/migration across the jars and a version + # claimed in any of them collides with managed-agent-server's exactly like + # an in-module duplicate. + flyway-migrations: + name: 'Flyway migration version uniqueness' + runs-on: 'ubuntu-latest' + timeout-minutes: 5 + steps: + - name: 'Checkout' + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 + + - name: 'Check Flyway migration versions are unique' + run: 'node scripts/check-flyway-migrations.js packages/sdk-java/managed-agent-server packages/sdk-java/runtime-broker packages/sdk-java/qwencode' + mysql-integration: name: 'Runtime Broker and Managed Agent MariaDB / Java 21' runs-on: 'ubuntu-latest' diff --git a/docs/design/2026-09-28-managed-agent-durable-lifecycle.md b/docs/design/2026-09-28-managed-agent-durable-lifecycle.md index 58e18681033..c6aa9f17931 100644 --- a/docs/design/2026-09-28-managed-agent-durable-lifecycle.md +++ b/docs/design/2026-09-28-managed-agent-durable-lifecycle.md @@ -294,7 +294,10 @@ W0e (#12839) took V16 first, so this migration is V17, the next free version on `main`. The O2 publication migrations follow as V18 and V19. An open pull request that takes V17 or a later version must renumber past it; a gap left instead would make Flyway refuse to start a database that already applied the -later version. Upgrade tests apply the ordered sequence before service startup. +later version. Upgrade tests apply the ordered sequence before service +startup. Uniqueness across both migration locations is enforced without a +database by `scripts/check-flyway-migrations.js`, which runs in the SDK Java +workflow on every pull request and push (#12940). ## 5. Tests diff --git a/docs/design/2026-09-28-managed-agent-durable-lifecycle.zh-CN.md b/docs/design/2026-09-28-managed-agent-durable-lifecycle.zh-CN.md index 7788e317fbf..8bc56f79273 100644 --- a/docs/design/2026-09-28-managed-agent-durable-lifecycle.zh-CN.md +++ b/docs/design/2026-09-28-managed-agent-durable-lifecycle.zh-CN.md @@ -136,7 +136,7 @@ V17 把每条待定的 `ARCHIVE_SESSION` 与 `DELETE_SESSION` 命令转换为待 不支持新旧版本混跑的滚动升级:V17 运行之前必须停掉所有旧版本服务器。升级之后由旧服务器留下的待定 archive 或 delete 不会被转换,它的待定命令会让该 Session 之后的所有生命周期变更都返回 `409 session_operation_active`;旧服务器的 unarchive 还会重新打开被 D4 关闭的 Session。 W0e(#12839)先占用了 V16,因此本迁移为 V17,即 `main` 上下一个空闲版本号。使用 V17 或更高版本的未合并 PR 必须重新编号到它之后;如果改为留出空号,已经应用了更高版本的数据库会被 Flyway 拒绝启动。 -本分支的 O2 publication 迁移接续使用 V18 和 V19。升级测试按此顺序应用迁移后再启动服务。 +本分支的 O2 publication 迁移接续使用 V18 和 V19。升级测试按此顺序应用迁移后再启动服务。两个迁移目录之间的版本号唯一性由 `scripts/check-flyway-migrations.js` 在不依赖数据库的情况下强制校验,它在 SDK Java workflow 的每个 pull request 与 push 上运行(#12940)。 ## 5. 测试 diff --git a/scripts/check-flyway-migrations.js b/scripts/check-flyway-migrations.js new file mode 100644 index 00000000000..bdc2081605b --- /dev/null +++ b/scripts/check-flyway-migrations.js @@ -0,0 +1,224 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// managed-agent-server allocates Flyway versions by hand across two +// directories that share one sequence — src/main/resources/db/migration (SQL) +// and src/main/java/db/migration (BaseJavaMigration classes). On 2026-09-28 +// two in-flight PRs each added a V16 and merged 23 minutes apart; each branch +// was green in isolation, and Flyway refused to boot the merged result +// ("Found more than one migration with version 16"), leaving main red for two +// hours (#12940). The test that catches a duplicate boots a database and runs +// only in the slow database-backed lanes, after the merge has landed. This +// check is the fast lane: it scans both locations and fails when two files +// claim the same version, naming them, in milliseconds and with no database. + +import { existsSync, readdirSync } from 'node:fs'; +import path from 'node:path'; +import { escapeWorkflowCommand } from './release-script-utils.js'; + +// A `::error::` command's data is runner-DECODED when the downloadable log +// is rendered, so escaping only CR/LF/% is not enough: the decode restores +// them exactly where the main-red consumer parses, and a directory or file +// name can carry a whole forged `::error::` line — a second flyway identity +// — or the `: N migrations claim version M: ` phrase that suppresses the +// genuine one from the same line. Nothing outside a migration path's real +// alphabet may reach the command data: every other byte becomes %XX first, +// with the shared helper's escape on top, so no decode pass can turn the +// text back into control bytes or pattern-shaped phrases. +const boundWorkflowPath = (text) => + escapeWorkflowCommand( + String(text).replace( + /[^A-Za-z0-9._/-]/g, + (char) => + `%${char.codePointAt(0).toString(16).toUpperCase().padStart(2, '0')}`, + ), + ); + +// Both locations resolve into Flyway's classpath:db/migration, so their +// versions share one namespace. Flyway scans each location AND its +// subdirectories, and nowhere else — a versioned-migration file anywhere +// else under the source root is invisible to it, so finding one there means +// the location moved and the guard must say so instead of passing on what is +// left. Everything below derives the db roots and source roots from this +// table, so editing it cannot silently blind the probes. +const LOCATIONS = [ + { dir: ['src', 'main', 'resources', 'db', 'migration'], suffix: '.sql' }, + { dir: ['src', 'main', 'java', 'db', 'migration'], suffix: '.java' }, +]; + +// A versioned migration is V__; the version is numeric +// segments joined by dots or underscores. +const MIGRATION_NAME = /^V(\d+(?:[._]\d+)*)__/; + +const modules = [...new Set(process.argv.slice(2))]; +if (modules.length === 0) { + console.error( + 'usage: node scripts/check-flyway-migrations.js ...', + ); + process.exit(2); +} + +// Flyway compares versions numerically segment by segment, so V016 collides +// with V16 and a trailing .0 segment carries no meaning. +const normalize = (version) => + version + .split(/[._]/) + .map((segment) => segment.replace(/^0+(?=\d)/, '')) + .join('.') + .replace(/(\.0)*$/, ''); + +// Flyway matches the suffix case-insensitively — V1__b.SQL claims version 1 +// exactly like V1__a.sql does. +const isMigrationFile = (name, suffix) => + name.toLowerCase().endsWith(suffix) && MIGRATION_NAME.test(name); + +// Not readdirSync's `recursive`: a Node older than 18.17 ignores it (see +// check-failsafe-reports.js). Flyway scans a location's subdirectories too. +const migrationFiles = (dir, suffix) => { + if (!existsSync(dir)) return []; + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => + entry.isDirectory() + ? migrationFiles(path.join(dir, entry.name), suffix) + : isMigrationFile(entry.name, suffix) + ? [path.join(dir, entry.name)] + : [], + ); +}; + +// Files shaped like a versioned migration of this location's kind, anywhere +// under the source root EXCEPT inside the configured location. A renamed +// location (db/migration → db/changelog) leaves the guard scanning an empty +// directory, so the renamed-away state must fail here — and so must a +// location whose files moved while its sibling stayed populated. +const outOfPlaceFiles = (module, location) => { + const sourceRoot = path.join(module, ...location.dir.slice(0, -2)); + const configured = path.join(module, ...location.dir); + const found = []; + const walk = (dir) => { + if (!existsSync(dir)) return; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const file = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (file !== configured) walk(file); + } else if (isMigrationFile(entry.name, location.suffix)) { + found.push(file); + } + } + }; + walk(sourceRoot); + return found; +}; + +// A db/migration* sibling under the location's OWN db root — including an +// emptied rename such as db/migrations — marks the location as moved even +// when nothing migration-shaped survives anywhere. A module with no such +// directory at all (runtime-broker today) simply has no sequence to check, +// which is what lets one invocation cover every module that shares the +// classpath namespace below. +const hasMigrationDir = (module, location) => { + const dbRoot = path.join(module, ...location.dir.slice(0, -1)); + return ( + existsSync(dbRoot) && + readdirSync(dbRoot, { withFileTypes: true }).some( + (entry) => + entry.isDirectory() && entry.name.startsWith(location.dir.at(-1)), + ) + ); +}; + +let failed = false; +// Flyway resolves classpath:db/migration across every jar on the classpath, +// and managed-agent-server depends on the other modules at compile scope, so +// the modules given in one invocation share one version namespace — a version +// claimed in two modules collides exactly like two files in one module do. +// Collisions are reported only after every module is scanned: one ::error:: +// line per collided version names EVERY claimant (the consumer keys its issue +// on the module and version in this line, so a second line for the same +// version would file a second issue), and a claimant module never prints an +// "all versions unique" summary. +const claimants = new Map(); +const scanned = []; +for (const module of modules) { + if (!existsSync(module)) { + failed = true; + console.error( + `::error::${boundWorkflowPath(module)}: no such Maven module directory`, + ); + continue; + } + let errored = false; + let count = 0; + for (const location of LOCATIONS) { + const files = migrationFiles( + path.join(module, ...location.dir), + location.suffix, + ); + // Out-of-place files must not join the claimants: invisible to Flyway, + // they cannot collide at runtime — they are the moved location's error, + // not a second claimant for their version. + const misplaced = outOfPlaceFiles(module, location); + if (misplaced.length > 0 && files.length > 0) { + // The location IS populated, so nothing moved: a stray file sits + // outside the only place Flyway scans. Say that — claiming the + // location is empty would send the reader hunting a rename that never + // happened, and the consumer titles the issue from this line. + failed = true; + errored = true; + console.error( + `::error::${boundWorkflowPath(module)}: found migration files outside ` + + `${location.dir.join('/')}: ` + + `${misplaced.map(boundWorkflowPath).join(', ')}`, + ); + } else if ( + misplaced.length > 0 || + // A location that EXISTS is not moved, however empty: the rename + // probe fires only when the location directory itself is gone and a + // migration* sibling took its place. + (files.length === 0 && + !existsSync(path.join(module, ...location.dir)) && + hasMigrationDir(module, location)) + ) { + failed = true; + errored = true; + console.error( + `::error::${boundWorkflowPath(module)}: found no migration under ` + + `${location.dir.join('/')}; if it moved, point this check at the ` + + `new location${ + misplaced.length > 0 + ? `: ${misplaced.map(boundWorkflowPath).join(', ')}` + : '' + }`, + ); + } + for (const file of files) { + count += 1; + const version = normalize(MIGRATION_NAME.exec(path.basename(file))[1]); + const group = claimants.get(version) ?? []; + group.push({ module, file }); + claimants.set(version, group); + } + } + if (!errored && count === 0) { + console.log(`${module}: no migration directories`); + continue; + } + scanned.push({ module, count, errored }); +} +const collided = new Set(); +for (const [version, group] of claimants) { + if (group.length < 2) continue; + failed = true; + console.error( + `::error::${boundWorkflowPath(group[0].module)}: ${group.length} migrations claim version ` + + `${escapeWorkflowCommand(version)}: ${group.map(({ file }) => boundWorkflowPath(file)).join(', ')}`, + ); + for (const { module } of group) collided.add(module); +} +for (const { module, count, errored } of scanned) { + if (errored || collided.has(module)) continue; + console.log(`${module}: ${count} migrations, all versions unique`); +} +process.exitCode = failed ? 1 : 0; diff --git a/scripts/generate-release-notes.js b/scripts/generate-release-notes.js index 0ca5e2b389d..b7dc488658c 100644 --- a/scripts/generate-release-notes.js +++ b/scripts/generate-release-notes.js @@ -9,7 +9,11 @@ import { execFileSync } from 'node:child_process'; import { appendFileSync, mkdirSync, writeFileSync } from 'node:fs'; import { dirname } from 'node:path'; -import { isMainModule, parseArgs } from './release-script-utils.js'; +import { + escapeWorkflowCommand, + isMainModule, + parseArgs, +} from './release-script-utils.js'; const GENERATED_ENTRY_RE = /^[*-]\s+(.+)\s+by\s+@([A-Za-z0-9-]+(?:\[bot\])?)((?:\s+with\s+@[A-Za-z0-9-]+(?:\[bot\])?)*)\s+in\s+(https?:\/\/\S+\/pull\/(\d+))\s*$/; @@ -1359,12 +1363,9 @@ async function main() { } } -export function escapeWorkflowCommand(text) { - return String(text) - .replace(/%/g, '%25') - .replace(/\r/g, '%0D') - .replace(/\n/g, '%0A'); -} +// Re-exported so existing importers of this module keep resolving; the +// implementation lives with the other release-script helpers. +export { escapeWorkflowCommand }; export function tryAppendDegradedStepSummary(result, summaryPath) { // The step summary is auxiliary; a filesystem failure there (EACCES, diff --git a/scripts/release-script-utils.js b/scripts/release-script-utils.js index a574915dbae..c17901b2c8b 100644 --- a/scripts/release-script-utils.js +++ b/scripts/release-script-utils.js @@ -50,6 +50,21 @@ export function parseSha256Sums(content) { return checksums; } +/** + * Escape a value embedded in a `::error::`/`::warning::` workflow command. + * The value can be contributor-controlled (a fork PR's filename, model + * output), and the runner parses workflow commands from stderr too: a raw LF + * would emit a second, forged command from the step. `%` must be replaced + * first, or the `%0A`/`%0D` sequences the other two arms emit get + * double-encoded. + */ +export function escapeWorkflowCommand(text) { + return String(text) + .replace(/%/g, '%25') + .replace(/\r/g, '%0D') + .replace(/\n/g, '%0A'); +} + export function readOptionValue(argv, index, optionName) { const value = argv[index + 1]; if (!value || value.startsWith('-')) { diff --git a/scripts/tests/check-flyway-migrations.test.js b/scripts/tests/check-flyway-migrations.test.js new file mode 100644 index 00000000000..e4d6aecc5f1 --- /dev/null +++ b/scripts/tests/check-flyway-migrations.test.js @@ -0,0 +1,409 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +const script = join( + dirname(fileURLToPath(import.meta.url)), + '..', + 'check-flyway-migrations.js', +); +let root; + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'check-flyway-migrations-')); +}); +afterEach(() => rmSync(root, { recursive: true, force: true })); + +// A Maven module holding the two migration locations that share one Flyway +// version sequence: SQL under resources, BaseJavaMigration classes under the +// db.migration package. +function module(name, { sql = [], java = [] } = {}) { + const dir = join(root, name); + for (const file of sql) { + const target = join(dir, 'src/main/resources/db/migration', file); + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, ''); + } + for (const file of java) { + const target = join(dir, 'src/main/java/db/migration', file); + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, ''); + } + return dir; +} + +function check(...args) { + const result = spawnSync(process.execPath, [script, ...args], { + encoding: 'utf8', + }); + return { status: result.status, output: result.stdout + result.stderr }; +} + +// The forgery fixtures below create filenames only the POSIX lanes allow +// (LF, `:`) — both un-creatable on the Windows lane, where this suite still +// runs. Gate on the CAPABILITY, not the platform, probing once at module +// scope; scripts/tests/review-artifact-upload.test.js sets the precedent. +const newlineNamesWork = (() => { + const probe = mkdtempSync(join(tmpdir(), 'flyway-name-probe-')); + try { + writeFileSync(join(probe, 'a\nb:c'), ''); + return true; + } catch { + return false; + } finally { + rmSync(probe, { recursive: true, force: true }); + } +})(); + +describe('check-flyway-migrations', () => { + it('passes when every version is claimed once across both locations', () => { + const dir = module('server', { + sql: ['V1__core.sql', 'V2__store.sql', 'V16__evidence.sql'], + java: ['V15__event_identity.java'], + }); + const result = check(dir); + expect(result.output).toContain('4 migrations, all versions unique'); + expect(result.status).toBe(0); + }); + + it('names both files when two migrations claim one version', () => { + const dir = module('server', { + sql: ['V16__runtime_loss_evidence.sql', 'V16__session_operation.sql'], + }); + const result = check(dir); + expect(result.output).toContain('2 migrations claim version 16'); + expect(result.output).toContain('V16__runtime_loss_evidence.sql'); + expect(result.output).toContain('V16__session_operation.sql'); + expect(result.output).not.toContain('all versions unique'); + expect(result.status).toBe(1); + }); + + it('sees a collision between the SQL and the Java location', () => { + const dir = module('server', { + sql: ['V15__event_identity.sql'], + java: ['V15__event_identity.java'], + }); + const result = check(dir); + expect(result.output).toContain('2 migrations claim version 15'); + expect(result.status).toBe(1); + }); + + it('sees a collision between two modules of one invocation', () => { + // Flyway resolves classpath:db/migration across every jar on the + // classpath, so a version claimed in two modules collides exactly like + // two files in one module do. + const first = module('first', { sql: ['V1__x.sql'] }); + const second = module('second', { sql: ['V1__y.sql'] }); + const result = check(first, second); + expect(result.output).toContain('2 migrations claim version 1'); + expect(result.output).toContain('V1__x.sql'); + expect(result.output).toContain('V1__y.sql'); + expect(result.status).toBe(1); + }); + + it('compares versions numerically the way Flyway does', () => { + const dir = module('server', { + sql: ['V016__a.sql', 'V16.0__c.sql', 'V16__b.sql'], + }); + const result = check(dir); + expect(result.output).toContain('3 migrations claim version 16'); + expect(result.status).toBe(1); + const distinct = module('distinct', { + sql: ['V16__a.sql', 'V16.1__b.sql'], + }); + expect(check(distinct).status).toBe(0); + }); + + it('equates the underscore a Java migration uses for a dotted version', () => { + // A Java class name cannot contain a dot, so a Java migration writes + // V1_1 for what SQL writes as V1.1; Flyway equates the two. + const dir = module('server', { + sql: ['V1.1__a.sql'], + java: ['V1_1__b.java'], + }); + const result = check(dir); + expect(result.output).toContain('2 migrations claim version 1.1'); + expect(result.status).toBe(1); + }); + + it('drops every trailing .0 segment, not only the last', () => { + const dir = module('server', { sql: ['V1__a.sql', 'V1.0.0__b.sql'] }); + const result = check(dir); + expect(result.output).toContain('2 migrations claim version 1'); + expect(result.status).toBe(1); + }); + + it('matches the migration suffix case-insensitively, as Flyway does', () => { + const dir = module('server', { sql: ['V1__a.sql', 'V1__b.SQL'] }); + const result = check(dir); + expect(result.output).toContain('2 migrations claim version 1'); + expect(result.status).toBe(1); + }); + + it('ignores a versioned name with a suffix Flyway does not scan', () => { + const dir = module('server', { sql: ['V1__a.sql', 'V1__b.txt'] }); + const result = check(dir); + expect(result.output).toContain('1 migrations, all versions unique'); + expect(result.status).toBe(0); + }); + + it('ignores files that are not versioned migrations', () => { + const dir = module('server', { + sql: [ + 'V1__core.sql', + 'notes.md', + 'R__refresh_view.sql', + 'V2_missing_a_separator.sql', + ], + java: ['package-info.java'], + }); + const result = check(dir); + expect(result.output).toContain('1 migrations, all versions unique'); + expect(result.status).toBe(0); + }); + + it('scans subdirectories, which Flyway scans too', () => { + const dir = module('server', { + sql: ['V1__core.sql', join('backfill', 'V1__again.sql')], + }); + const result = check(dir); + expect(result.output).toContain('2 migrations claim version 1'); + expect(result.status).toBe(1); + }); + + it('checks every module it is given', () => { + const first = module('first', { sql: ['V1__a.sql'] }); + const second = module('second', { sql: ['V2__b.sql', 'V2__c.sql'] }); + const result = check(first, second); + expect(result.output).toContain('first: 1 migrations, all versions unique'); + expect(result.output).toContain('second: 2 migrations claim version 2'); + expect(result.status).toBe(1); + }); + + it('fails when a location rename would empty the guard', () => { + const dir = module('server', { sql: ['V1__a.sql'] }); + rmSync(join(dir, 'src'), { recursive: true }); + mkdirSync(join(dir, 'src/main/resources/db/migrations'), { + recursive: true, + }); + const result = check(dir); + expect(result.output).toContain('found no migration under'); + expect(result.status).toBe(1); + }); + + it('fails when the SQL location moved but the Java location stayed', () => { + // Only one of the two locations moving must not pass vacuously: the + // aggregate count still sees the Java migration, but the renamed-away + // SQL population — the one #12940 collided in — is no longer scanned. + const dir = module('server', { java: ['V15__event_identity.java'] }); + const moved = join(dir, 'src/main/resources/db/migrations'); + mkdirSync(moved, { recursive: true }); + writeFileSync(join(moved, 'V16__a.sql'), ''); + writeFileSync(join(moved, 'V16__b.sql'), ''); + const result = check(dir); + expect(result.output).toContain( + 'found no migration under src/main/resources/db/migration', + ); + // A moved location is an error, not a smaller scan: no clean summary. + expect(result.output).not.toContain('all versions unique'); + expect(result.status).toBe(1); + }); + + it('fails when the Java location moved but the SQL location stayed', () => { + // The twin of the case above, and the live layout: managed-agent-server + // really carries its V15 under src/main/java/db/migration, so a package + // rename there must fail rather than print "all versions unique" from a + // scan that skipped it. + const dir = module('server', { sql: ['V15__a.sql'] }); + const moved = join(dir, 'src/main/java/db/migrations'); + mkdirSync(moved, { recursive: true }); + writeFileSync(join(moved, 'V15__b.java'), ''); + const result = check(dir); + expect(result.output).toContain( + 'found no migration under src/main/java/db/migration', + ); + expect(result.output).not.toContain('all versions unique'); + expect(result.status).toBe(1); + }); + + it('fails when the location was renamed outside the migration* family', () => { + // The ownership predicate cannot key on the two exact directory names: + // db/changelog holds the whole sequence, no db/migration* sibling exists + // anywhere, and a name-keyed probe passes silently on exactly the + // collision the guard exists for. + const dir = module('server', { sql: ['V16__a.sql', 'V16__b.sql'] }); + rmSync(join(dir, 'src/main/resources/db/migration'), { recursive: true }); + const moved = join(dir, 'src/main/resources/db/changelog'); + mkdirSync(moved, { recursive: true }); + writeFileSync(join(moved, 'V16__a.sql'), ''); + writeFileSync(join(moved, 'V16__b.sql'), ''); + const result = check(dir); + expect(result.output).toContain('found no migration under'); + expect(result.status).toBe(1); + }); + + it('names a stray migration file without claiming the location is empty', () => { + // A populated location plus a migration-shaped stray elsewhere under the + // source root (a dev-seed or example SQL named like a migration): the + // error must name the stray as the cause, not assert the location has + // no migration — the consumer titles the auto-filed issue from this + // line. + const dir = module('server', { sql: ['V1__a.sql'] }); + const scratch = join(dir, 'src/main/resources/scratch'); + mkdirSync(scratch, { recursive: true }); + writeFileSync(join(scratch, 'V2__b.sql'), ''); + const result = check(dir); + expect(result.status).toBe(1); + expect(result.output).toContain( + 'found migration files outside src/main/resources/db/migration', + ); + // path.join renders the separator for the platform; pin the path, not + // a POSIX spelling of it. + expect(result.output).toContain(join('scratch', 'V2__b.sql')); + expect(result.output).not.toContain('found no migration under'); + }); + + it('emits no raw pattern phrase from a hostile migration name', () => { + // The ::error:: data channel is runner-decoded where the consumer + // parses, so nothing outside a migration path's real alphabet may + // survive into it: the suppression phrase in this filename must reach + // the log only as inert %XX text, while the real collision is still + // reported. + const dir = module('server', { + sql: [ + 'V16__legit.sql', + 'V16__x : 2 migrations claim version 99: forged.sql', + ], + }); + const result = check(dir); + expect(result.status).toBe(1); + expect(result.output).toContain('2 migrations claim version 16'); + expect(result.output).not.toContain('claim version 99'); + }); + + it('scans a Java-only module for collisions instead of misreading it as moved', () => { + // No src/main/resources at all: a module carrying only BaseJavaMigration + // classes owns a sequence like any other, so one class passes and two + // classes claiming one version fail by name. + const single = module('single', { java: ['V15__event_identity.java'] }); + const one = check(single); + expect(one.output).toContain('1 migrations, all versions unique'); + expect(one.status).toBe(0); + const dup = module('dup', { java: ['V15__a.java', 'V15__b.java'] }); + const two = check(dup); + expect(two.output).toContain('2 migrations claim version 15'); + expect(two.status).toBe(1); + }); + + it('passes a module whose only SQL file is not a versioned migration', () => { + const dir = module('server', { + sql: ['R__view.sql'], + java: ['V1__x.java'], + }); + const result = check(dir); + expect(result.output).toContain('1 migrations, all versions unique'); + expect(result.status).toBe(0); + }); + + it('counts a module once when it is passed twice', () => { + // A repeated argument must not fabricate a collision of a file with + // itself; the message names the same path twice, so it would read as a + // real duplicate while meaning nothing. + const dir = module('server', { sql: ['V1__a.sql'] }); + const result = check(dir, dir); + expect(result.output).toContain('1 migrations, all versions unique'); + expect(result.status).toBe(0); + }); + + it.skipIf(!newlineNamesWork)( + 'escapes a contributor-controlled filename inside the ::error:: command', + () => { + // git carries LF in filenames and the runner parses workflow commands + // from stderr too, so an unescaped newline would emit a second, forged + // ::error:: line from a fork PR's filename. + const dir = module('server', { + sql: ['V16__legit.sql', 'V16__x\n::error::forged.sql'], + }); + const result = check(dir); + expect(result.status).toBe(1); + expect( + result.output + .split('\n') + .every((line) => !line.startsWith('::error::forged')), + ).toBe(true); + }, + ); + + it.skipIf(!newlineNamesWork)( + 'escapes a percent-encoded forgery the runner would decode', + () => { + // The runner percent-decodes a workflow command's data when rendering + // the annotation, so the forgery this defends against appears only + // AFTER decoding: an unescaped %0A in a filename renders as a second + // ::error:: line no file contained, while the raw-byte oracle above + // still passes. Decode the way the runner does — %25 LAST, or the + // %0A/%0D the escaper emitted would decode twice. + const dir = module('server', { + sql: ['V16__legit.sql', 'V16__x%0A::error::forged.sql'], + }); + const result = check(dir); + expect(result.status).toBe(1); + const decoded = result.output + .replace(/%0D/g, '\r') + .replace(/%0A/g, '\n') + .replace(/%25/g, '%'); + expect( + decoded + .split('\n') + .every((line) => !line.startsWith('::error::forged')), + ).toBe(true); + }, + ); + + it('reports a three-way collision once, naming every claimant', () => { + // One version claimed in three modules of one invocation is ONE + // collision: the consumer keys its issue on the module and version in + // this line, so a second line naming another module would file a second + // issue — and a claimant must never print a clean summary. + const first = module('first', { sql: ['V1__x.sql'] }); + const second = module('second', { sql: ['V1__y.sql'] }); + const third = module('third', { sql: ['V1__z.sql'] }); + const result = check(first, second, third); + expect(result.status).toBe(1); + const errorLines = result.output + .split('\n') + .filter((line) => line.startsWith('::error::')); + expect(errorLines).toHaveLength(1); + expect(errorLines[0]).toContain('3 migrations claim version 1'); + expect(errorLines[0]).toContain('V1__x.sql'); + expect(errorLines[0]).toContain('V1__y.sql'); + expect(errorLines[0]).toContain('V1__z.sql'); + expect(result.output).not.toContain('all versions unique'); + }); + + it('passes a module that owns no migrations', () => { + // runtime-broker shares the classpath namespace but carries no migration + // sequence; requiring its SQL location would fail the whole invocation. + const dir = module('broker'); + mkdirSync(join(dir, 'src/main/java/com/example'), { recursive: true }); + const result = check(dir); + expect(result.output).toContain('no migration directories'); + expect(result.status).toBe(0); + }); + + it('refuses a missing module or a missing argument', () => { + expect(check(join(root, 'absent')).status).toBe(1); + const bare = check(); + expect(bare.output).toContain('usage:'); + expect(bare.status).toBe(2); + }); +}); diff --git a/scripts/tests/get-release-version.test.js b/scripts/tests/get-release-version.test.js index 7ebbdeedf6f..00b8fff8324 100644 --- a/scripts/tests/get-release-version.test.js +++ b/scripts/tests/get-release-version.test.js @@ -894,8 +894,8 @@ describe('assertVersionUnreleased', () => { return notFoundAnywhere(command); }); vi.spyOn(console, 'error').mockImplementation(() => {}); - // The runner parses workflow commands from stdout only; ::error:: on - // stderr would never surface as an annotation in the Actions UI. + // The runner parses workflow commands from both streams; this CLI emits + // ::error:: on stdout. const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); expect(runAssertVersionCli('1.2.3')).toBe(3); expect(logSpy).toHaveBeenCalledWith( diff --git a/scripts/tests/main-ci-failure-issue-workflow.test.js b/scripts/tests/main-ci-failure-issue-workflow.test.js index 5131f650f88..334ad7b916d 100644 --- a/scripts/tests/main-ci-failure-issue-workflow.test.js +++ b/scripts/tests/main-ci-failure-issue-workflow.test.js @@ -22,8 +22,13 @@ describe('main CI failure issue workflow', () => { it('opens an autofix-ready issue only for failed main CI runs', () => { expect(workflow).toContain('workflow_run:'); + // 'SDK Java' joined for its post-merge push run: its path filter watches + // the SDK's embedding surfaces too, so it fires on roughly half of recent + // merges to main — and a red push run is the only signal for a merge + // result neither PR could fail: the duplicate-V16 Flyway collision of + // #12940 sat unnoticed for two hours without it. expect(workflow).toContain( - "workflows: ['E2E Tests', 'SDK Python', 'Qwen Code CI']", + "workflows: ['E2E Tests', 'SDK Java', 'SDK Python', 'Qwen Code CI']", ); expect(workflow).toContain("types: ['completed']"); // 'Qwen Code CI' joined the list when the macOS and Windows lanes got a @@ -59,6 +64,17 @@ describe('main CI failure issue workflow', () => { ); }); + it('pins the watched name to the name key of sdk-java.yml', () => { + // `workflow_run.workflows` matches the watched workflow's `name:` key, not + // its filename — renaming sdk-java.yml's name must fail here instead of + // silently stopping the SDK Java failure issues. + const sdkJava = parse( + readFileSync('.github/workflows/sdk-java.yml', 'utf8'), + ); + expect(sdkJava.name).toBe('SDK Java'); + expect(yml.on.workflow_run.workflows).toContain(sdkJava.name); + }); + it('creates an issue that the existing autofix worker can pick up', () => { expect(workflow).toContain("issues: 'write'"); expect(workflow).toContain('CI_DEV_BOT_PAT'); diff --git a/scripts/tests/sdk-java-workflow.test.js b/scripts/tests/sdk-java-workflow.test.js index b81f5758175..82adca32ef5 100644 --- a/scripts/tests/sdk-java-workflow.test.js +++ b/scripts/tests/sdk-java-workflow.test.js @@ -1,4 +1,6 @@ -import { readFileSync } from 'node:fs'; +import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { parse } from 'yaml'; import { describe, expect, it } from 'vitest'; const workflow = readFileSync('.github/workflows/sdk-java.yml', 'utf8'); @@ -84,3 +86,54 @@ describe('SDK Java self-hosted workflow guards', () => { }, ); }); + +// #12940: the duplicate-version guard is the fast lane for a collision two +// green PRs can only produce in the merge result, so it runs on every +// trigger — no self-hosted routing, no Java, no database. +describe('SDK Java Flyway migration version guard', () => { + it('runs the uniqueness check as an unconditional job', () => { + const block = job('flyway-migrations'); + expect(block).toContain("runs-on: 'ubuntu-latest'"); + expect(block).toContain('actions/checkout@'); + expect(block).not.toContain('if:'); + expect(block).toContain( + "run: 'node scripts/check-flyway-migrations.js packages/sdk-java/managed-agent-server packages/sdk-java/runtime-broker packages/sdk-java/qwencode'", + ); + }); + + it('scans every sdk-java module that owns a db/migration directory', () => { + // The guard's premise is the shared classpath:db/migration namespace — + // managed-agent-server depends on runtime-broker, so one invocation must + // name every module that owns a migration sequence. Derive the + // expectation from the tree: a module that grows a db/migration without + // joining the invocation turns this red. + const owners = new Set(); + for (const entry of readdirSync('packages/sdk-java', { + withFileTypes: true, + })) { + if (!entry.isDirectory()) continue; + for (const location of [ + 'src/main/resources/db/migration', + 'src/main/java/db/migration', + ]) { + if (existsSync(join('packages/sdk-java', entry.name, location))) { + owners.add(`packages/sdk-java/${entry.name}`); + } + } + } + expect(owners.size).toBeGreaterThan(0); + const block = job('flyway-migrations'); + for (const owner of owners) { + expect(block).toContain(owner); + } + }); + + it('triggers the workflow when the guard script itself changes', () => { + const yml = parse(workflow); + for (const event of ['pull_request', 'push']) { + expect(yml.on[event].paths).toContain( + 'scripts/check-flyway-migrations.js', + ); + } + }); +});