From e0ce95c592c00f100f5141371dbda976d678ddee Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Sat, 8 Aug 2026 23:24:11 -0300 Subject: [PATCH] fix(ci): close remaining release-green gaps (#9835) Co-authored-by: diegosouzapw --- scripts/quality/validate-release-green.mjs | 43 +++++++++--- .../session-dedup-memory-7849.test.ts | 2 + ...e-limit-queue-timeout-message-4165.test.ts | 52 +++++++++----- tests/unit/search-route.test.ts | 6 ++ .../unit/translator-claude-to-gemini.test.ts | 10 +++ tests/unit/validate-release-green.test.ts | 67 ++++++++++++++----- 6 files changed, 139 insertions(+), 41 deletions(-) diff --git a/scripts/quality/validate-release-green.mjs b/scripts/quality/validate-release-green.mjs index f9693aa92e..05690e1eac 100644 --- a/scripts/quality/validate-release-green.mjs +++ b/scripts/quality/validate-release-green.mjs @@ -609,14 +609,6 @@ async function main() { args: ["run", "check:pack-artifact"], timeout: 20 * 60 * 1000, }); - // WS1.2 (#7065 class): boot the REAL packed tarball from a clean install — - // the runtime gate structure checks cannot provide. Reuses the same dist/ build. - slow.push({ - id: "pack-boot", - label: "Tarball boot-smoke (installed CLI serves /health)", - args: ["run", "check:pack-boot"], - timeout: 15 * 60 * 1000, - }); } slow.forEach((g) => announce(`${g.label} [parallel]`)); const slowResults = await Promise.all( @@ -633,6 +625,41 @@ async function main() { detail: code === 0 ? "pass" : firstFailureLine(out), }); }); + + if (WITH_BUILD) { + // WS1.2 (#7065 class): boot the REAL packed tarball from a clean install. + // check:pack-artifact is the builder for dist/ when staging is absent, so the + // boot smoke MUST run after it completes. Running both in the parallel wave + // races check:pack-boot against dist/server.js creation on clean worktrees. + const packArtifactIndex = slow.findIndex((g) => g.id === "pack-artifact"); + const packArtifactResult = slowResults[packArtifactIndex]; + const bootLabel = "Tarball boot-smoke (installed CLI serves /health)"; + + if (!packArtifactResult || packArtifactResult.code !== 0) { + const out = "skipped because package-artifact did not produce a valid dist/ build"; + saveGateLog("pack-boot", out); + record({ + id: "pack-boot", + label: bootLabel, + kind: "hard", + ok: false, + detail: out, + }); + } else { + announce(bootLabel); + const { code, out } = await runAsync(npmCmd, ["run", "check:pack-boot"], { + timeout: 15 * 60 * 1000, + }); + saveGateLog("pack-boot", out); + record({ + id: "pack-boot", + label: bootLabel, + kind: "hard", + ok: code === 0, + detail: code === 0 ? "pass" : firstFailureLine(out), + }); + } + } } else if (WITH_BUILD) { // --with-build without the suites (--quick): still verify the package artifact. const { code, out } = await runAsync(npmCmd, ["run", "check:pack-artifact"], { diff --git a/tests/unit/compression/session-dedup-memory-7849.test.ts b/tests/unit/compression/session-dedup-memory-7849.test.ts index dd3e6c2d98..8cab226280 100644 --- a/tests/unit/compression/session-dedup-memory-7849.test.ts +++ b/tests/unit/compression/session-dedup-memory-7849.test.ts @@ -80,6 +80,8 @@ test("#7849: the two-message pathological pair stays bounded", () => { Date.now() - started < 4000, "the pathological pair must stay fast; quadratic work would take seconds" ); + assert.strictEqual(result.body, body, "bounded processing must preserve the input body"); + assert.equal(result.compressed, false, "the non-deduplicable pair must fail open"); assert.ok(Array.isArray((result.body as { messages?: unknown[] }).messages)); }); diff --git a/tests/unit/rate-limit-queue-timeout-message-4165.test.ts b/tests/unit/rate-limit-queue-timeout-message-4165.test.ts index e514bb9428..685c01e67d 100644 --- a/tests/unit/rate-limit-queue-timeout-message-4165.test.ts +++ b/tests/unit/rate-limit-queue-timeout-message-4165.test.ts @@ -26,6 +26,12 @@ function wait(ms: number) { return new Promise((resolve) => setTimeout(resolve, ms)); } +// Leave enough scheduling headroom for a loaded CI/devbox while keeping the +// executing callback longer than the queue-only budget. The actual queued-job +// case stays short because it controls dispatch deterministically. +const DISPATCHED_QUEUE_BUDGET_MS = 2_000; +const QUEUED_QUEUE_BUDGET_MS = 250; + test.afterEach(async () => { await rateLimitManager.__resetRateLimitManagerForTests(); }); @@ -43,14 +49,23 @@ async function triggerQueueTimeout() { concurrentRequests: 1, requestsPerMinute: 100000, minTimeBetweenRequestsMs: 0, - maxWaitMs: 40, + maxWaitMs: DISPATCHED_QUEUE_BUDGET_MS, }); - rateLimitManager.enableRateLimitProtection("conn-queue-timeout"); + const connectionId = "conn-dispatched-timeout"; + rateLimitManager.enableRateLimitProtection(connectionId); - return rateLimitManager.withRateLimit("openai", "conn-queue-timeout", "gpt-4o", async () => { - await wait(400); // > maxWaitMs (40ms) → Bottleneck fails the job - return "should-not-reach"; - }); + let dispatched = false; + const result = await rateLimitManager.withRateLimit( + "test-provider", + connectionId, + null, + async () => { + dispatched = true; + await wait(DISPATCHED_QUEUE_BUDGET_MS + 250); + return "should-not-reach"; + } + ); + return { dispatched, result }; } async function triggerQueuedTimeout() { @@ -60,7 +75,7 @@ async function triggerQueuedTimeout() { concurrentRequests: 1, requestsPerMinute: 0, minTimeBetweenRequestsMs: 0, - maxWaitMs: 40, + maxWaitMs: QUEUED_QUEUE_BUDGET_MS, }); const connectionId = "conn-queued-timeout"; rateLimitManager.enableRateLimitProtection(connectionId); @@ -79,13 +94,12 @@ async function triggerQueuedTimeout() { await firstExecuting; let caught: unknown; + let queuedDispatched = false; try { - await rateLimitManager.withRateLimit( - "test-provider", - connectionId, - null, - async () => "should-not-dispatch" - ); + await rateLimitManager.withRateLimit("test-provider", connectionId, null, async () => { + queuedDispatched = true; + return "should-not-dispatch"; + }); assert.fail("expected the queued job to expire"); } catch (error) { caught = error; @@ -93,16 +107,20 @@ async function triggerQueuedTimeout() { releaseFirst(); await first; } - return caught; + return { caught, queuedDispatched }; } test("#4165 a dispatched provider call is not killed by the queue budget", async () => { - const result = await triggerQueueTimeout(); - assert.equal(result, "should-not-reach"); + const execution = await triggerQueueTimeout(); + assert.equal(execution.dispatched, true, "the callback must enter execution"); + assert.equal(execution.result, "should-not-reach"); }); test("#4165 queue expiry surfaces a clear local error", async () => { - const caught = (await triggerQueuedTimeout()) as Error & { code?: string }; + const result = await triggerQueuedTimeout(); + assert.ok(result.caught instanceof Error, "queue expiry must reject with an Error"); + assert.equal(result.queuedDispatched, false, "an expired queued callback must never dispatch"); + const caught = result.caught as Error & { code?: string }; assert.equal(caught.code, "RATE_LIMIT_QUEUE_TIMEOUT"); assert.match(caught.message, /maxWaitMs/); assert.match(caught.message, /not an upstream/i); diff --git a/tests/unit/search-route.test.ts b/tests/unit/search-route.test.ts index ce634b4877..4cb231a64b 100644 --- a/tests/unit/search-route.test.ts +++ b/tests/unit/search-route.test.ts @@ -452,7 +452,13 @@ test("v1 search POST returns 400 when auto-select finds no configured provider ( const body = (await response.json()) as any; assert.equal(response.status, 400); + assert.equal(capturedUrl, "", "fallback-only SearXNG must not receive an upstream request"); assert.ok(body.error?.message || body.error); + assert.match( + String(body.error?.message ?? body.error), + /provider|configured/i, + "the response must explain that no provider was selected" + ); } finally { globalThis.fetch = originalFetch; } diff --git a/tests/unit/translator-claude-to-gemini.test.ts b/tests/unit/translator-claude-to-gemini.test.ts index 0fd757b8e3..86ada46d8c 100644 --- a/tests/unit/translator-claude-to-gemini.test.ts +++ b/tests/unit/translator-claude-to-gemini.test.ts @@ -200,6 +200,16 @@ test("Claude -> Gemini omits unsigned functionCall instead of injecting a fake t false, "signature-less tool_use must not become a native functionCall" ); + assert.equal( + JSON.stringify(result).includes('"thoughtSignature"'), + false, + "the translator must not synthesize a fake thought signature" + ); + assert.equal( + JSON.stringify(result).includes("read_file"), + false, + "the omitted unsigned call must not leak its tool payload elsewhere" + ); }); test("Claude -> Gemini sanitizes long tool names and exposes a restore map", () => { diff --git a/tests/unit/validate-release-green.test.ts b/tests/unit/validate-release-green.test.ts index bd37c75ae5..5b5b20a60a 100644 --- a/tests/unit/validate-release-green.test.ts +++ b/tests/unit/validate-release-green.test.ts @@ -22,16 +22,12 @@ const extract = extractCiGates as ( ) => { id: string; job: string; args: string[]; env?: Record }[]; test("eslintCounts sums errors + warnings across files", () => { - const parsed = [ - { errorCount: 2, warningCount: 5 }, - { errorCount: 0, warningCount: 3 }, - {}, - ]; + const parsed = [{ errorCount: 2, warningCount: 5 }, { errorCount: 0, warningCount: 3 }, {}]; assert.deepEqual(eslintCounts(parsed), { errors: 2, warnings: 8 }); }); test("parseEslintJson tolerates a leading non-JSON banner", () => { - const out = "npm warn something\n[{\"errorCount\":0,\"warningCount\":1}]"; + const out = 'npm warn something\n[{"errorCount":0,"warningCount":1}]'; assert.deepEqual(parseEslintJson(out), [{ errorCount: 0, warningCount: 1 }]); assert.equal(parseEslintJson("no json here"), null); }); @@ -52,8 +48,14 @@ test("parseEslintJson tolerates ESLint's trailing unpruned-suppressions stderr s }); test("parseCognitiveCount reads the gate's count (en + pt)", () => { - assert.equal(parseCognitiveCount("[cognitive-complexity] 797 function(s) exceed the threshold (15)."), 797); - assert.equal(parseCognitiveCount("[cognitive-complexity] REGRESSÃO — 801 violações > baseline 797"), 801); + assert.equal( + parseCognitiveCount("[cognitive-complexity] 797 function(s) exceed the threshold (15)."), + 797 + ); + assert.equal( + parseCognitiveCount("[cognitive-complexity] REGRESSÃO — 801 violações > baseline 797"), + 801 + ); assert.equal(parseCognitiveCount("no number"), null); }); @@ -175,8 +177,16 @@ test("pre-flight wires the test-masking PR-context gate against origin/main (v3. ); // run() must honor a per-gate env override so GITHUB_BASE_REF actually reaches the child // (routed through buildGateEnv since the --hermetic scrub was added). - assert.match(src, /env:\s*buildGateEnv\(opts\.env\)/, "run() must merge opts.env into the child env"); - assert.match(src, /\.\.\.\(extra \|\| \{\}\)/, "buildGateEnv must spread the per-gate env override"); + assert.match( + src, + /env:\s*buildGateEnv\(opts\.env\)/, + "run() must merge opts.env into the child env" + ); + assert.match( + src, + /\.\.\.\(extra \|\| \{\}\)/, + "buildGateEnv must spread the per-gate env override" + ); }); test("pre-flight --hermetic scrubs the live-test trigger vars (2026-07-05 false-positive fix)", async () => { @@ -214,6 +224,27 @@ test("pre-flight runs the slow suites CONCURRENTLY (v3.8.45 perf — was ~1h ser assert.match(src, /slow\.forEach\([\s\S]*?saveGateLog\(g\.id/, "each slow gate persists its log"); }); +test("pre-flight runs tarball boot only after the package artifact builder completes", async () => { + const fs = await import("node:fs"); + const src = fs.readFileSync( + new URL("../../scripts/quality/validate-release-green.mjs", import.meta.url), + "utf8" + ); + const parallelWave = src.indexOf("const slowResults = await Promise.all"); + const packBoot = src.indexOf('id: "pack-boot"'); + + assert.ok(parallelWave >= 0, "the parallel slow-gate wave must exist"); + assert.ok( + packBoot > parallelWave, + "pack-boot must be declared after the parallel artifact build" + ); + assert.match( + src, + /packArtifactResult[\s\S]*?check:pack-boot/, + "pack-boot must be explicitly sequenced from the package-artifact result" + ); +}); + // ─── --full-ci gate extraction (P0, v3.8.46 post-mortem) ───────────────────── const CI_FIXTURE = ` @@ -259,7 +290,11 @@ test("extractCiGates: pulls npm-run gate steps from the ci.yml gate jobs only", assert.ok(ids.includes("check:docs-all") && ids.includes("check:docs-symbols"), "multi-line run"); // …and NON-gate steps + jobs outside the gate set are ignored. assert.ok(!ids.includes("build") && !ids.some((i) => i.startsWith("test:")), "no build/test-run"); - assert.equal(gates.find((g) => g.job === "test-unit"), undefined, "test-unit job is not scanned"); + assert.equal( + gates.find((g) => g.job === "test-unit"), + undefined, + "test-unit job is not scanned" + ); }); test("extractCiGates: preserves `-- ` so ratchet flags reach the script", () => { @@ -272,7 +307,10 @@ test("extractCiGates: preserves `-- ` so ratchet flags reach the script", test("extractCiGates: skips the non-local gates (pr-evidence, codeql-ratchet)", () => { const ids = extract(CI_FIXTURE).map((g) => g.id); assert.ok(!ids.includes("check:pr-evidence"), "pr-evidence needs a PR body — skipped"); - assert.ok(!ids.includes("check:codeql-ratchet"), "codeql-ratchet is a remote-main check — skipped"); + assert.ok( + !ids.includes("check:codeql-ratchet"), + "codeql-ratchet is a remote-main check — skipped" + ); assert.ok(FULL_CI_SKIP.has("check:pr-evidence") && FULL_CI_SKIP.has("check:codeql-ratchet")); }); @@ -295,10 +333,7 @@ test("extractCiGates: attaches GITHUB_BASE_REF=main env to test-masking + de-dup test("extractCiGates: the REAL ci.yml yields the base-reds that leaked in v3.8.46", async () => { const fs = await import("node:fs"); - const yaml = fs.readFileSync( - new URL("../../.github/workflows/ci.yml", import.meta.url), - "utf8" - ); + const yaml = fs.readFileSync(new URL("../../.github/workflows/ci.yml", import.meta.url), "utf8"); const ids = new Set(extract(yaml).map((g) => g.id)); // The exact gates that leaked to the v3.8.46 release PR because the pre-flight // never ran them — --full-ci now reproduces every one.