fix(release): resolve inherited base-reds surfaced by v3.8.35 release CI

Cycle base-reds that only run on PR→main (not the PR→release fast-path):

- test(autoCombo): suffixComposition-4517 used node:test in a vitest-only dir
  (#4753) → vitest found no suite. Switch to the vitest API. (Vitest job)
- test(agentSkills): openapiParser fixture wrote docs/reference/openapi.yaml;
  parser reads docs/openapi.yaml since #4781 → point fixture at the new path.
  (Unit/Coverage/Node24/Node26 shard 4)
- test(integration): proxy-pipeline source-scan expected inline streaming-cost
  code that #4790/#3501 extracted to the recordStreamingCost leaf → assert the
  delegation instead. (Integration 1/2)
- fix(chatCore): derive the log trace id from crypto, not Math.random
  (CodeQL js/insecure-randomness — log-correlation id, not a secret).
- test(resilience): circuit-breaker invalid-cooldown fallback asserted t>29000,
  flaking on slow CI where ~1.6s elapsed gave t=28401 → tolerate wall-clock
  drift (t>25000). (Unit 6/8)
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-23 15:54:05 -03:00
parent 92175a397f
commit 37c49781a7
5 changed files with 90 additions and 48 deletions

View File

@@ -353,8 +353,10 @@ export async function handleChatCore({
const isModelScope = () => isModelScopeProvider(provider, credentials?.providerSpecificData); const isModelScope = () => isModelScopeProvider(provider, credentials?.providerSpecificData);
const startTime = Date.now(); const startTime = Date.now();
// Per-request trace id + checkpoint helper. Lets us see exactly which await // Per-request trace id + checkpoint helper. Lets us see exactly which await
// a hung request was sitting on in `[STAGE_TRACE]` log lines. // a hung request was sitting on in `[STAGE_TRACE]` log lines. Uses crypto RNG
const traceId = Math.random().toString(36).slice(2, 8); // (not Math.random) purely to satisfy CodeQL js/insecure-randomness — this id
// is a log-correlation token, not a security secret.
const traceId = globalThis.crypto.randomUUID().slice(0, 6);
// Emit request.started event for real-time dashboard // Emit request.started event for real-time dashboard
setImmediate(() => { setImmediate(() => {
@@ -1126,20 +1128,21 @@ export async function handleChatCore({
} }
} }
// Phase 4A: unified output styles (supersedes cavemanOutputMode via the back-compat shim). // Phase 4A: unified output styles (supersedes cavemanOutputMode via the back-compat shim).
let outputStyleResult: import("../services/compression/outputStyles/apply.ts").OutputStylesResult | null = let outputStyleResult:
null; | import("../services/compression/outputStyles/apply.ts").OutputStylesResult
| null = null;
if (config.enabled) { if (config.enabled) {
try { try {
const { resolveOutputStyleSelection } = await import( const { resolveOutputStyleSelection } =
"../services/compression/outputStyles/backCompat.ts" await import("../services/compression/outputStyles/backCompat.ts");
);
const selection = resolveOutputStyleSelection(config); const selection = resolveOutputStyleSelection(config);
if (selection.length > 0) { if (selection.length > 0) {
const { applyOutputStyles } = await import( const { applyOutputStyles } =
"../services/compression/outputStyles/apply.ts" await import("../services/compression/outputStyles/apply.ts");
);
const outputStyleLanguage = const outputStyleLanguage =
config.languageConfig?.enabled === true ? config.languageConfig.defaultLanguage : "en"; config.languageConfig?.enabled === true
? config.languageConfig.defaultLanguage
: "en";
outputStyleResult = applyOutputStyles( outputStyleResult = applyOutputStyles(
body as Parameters<typeof applyOutputStyles>[0], body as Parameters<typeof applyOutputStyles>[0],
selection, selection,
@@ -1156,7 +1159,10 @@ export async function handleChatCore({
outputStyleResult.skippedReason && outputStyleResult.skippedReason &&
outputStyleResult.skippedReason !== "no_styles" outputStyleResult.skippedReason !== "no_styles"
) { ) {
log?.debug?.("COMPRESSION", `Output styles skipped: ${outputStyleResult.skippedReason}`); log?.debug?.(
"COMPRESSION",
`Output styles skipped: ${outputStyleResult.skippedReason}`
);
} }
} }
} catch (err) { } catch (err) {
@@ -1175,7 +1181,9 @@ export async function handleChatCore({
typeof (compressionInputBody as Record<string, unknown>)?.max_tokens === "number" typeof (compressionInputBody as Record<string, unknown>)?.max_tokens === "number"
? ((compressionInputBody as Record<string, unknown>).max_tokens as number) ? ((compressionInputBody as Record<string, unknown>).max_tokens as number)
: null; : null;
let adaptiveTelemetry: import("../services/compression/adaptiveCompression/types.ts").AdaptiveTelemetry | null = null; let adaptiveTelemetry:
| import("../services/compression/adaptiveCompression/types.ts").AdaptiveTelemetry
| null = null;
const compressionPlan = selectCompressionPlan( const compressionPlan = selectCompressionPlan(
config, config,
compressionComboKey, compressionComboKey,
@@ -1187,7 +1195,9 @@ export async function handleChatCore({
{ {
modelContextLimit: adaptiveModelContextLimit, modelContextLimit: adaptiveModelContextLimit,
requestMaxTokens: requestMaxTokens, requestMaxTokens: requestMaxTokens,
onAdaptive: (t) => { adaptiveTelemetry = t; }, onAdaptive: (t) => {
adaptiveTelemetry = t;
},
} }
); );
const mode = compressionPlan.mode as CompressionConfig["defaultMode"]; const mode = compressionPlan.mode as CompressionConfig["defaultMode"];
@@ -1400,12 +1410,10 @@ export async function handleChatCore({
if (outputStyleResult) { if (outputStyleResult) {
void (async () => { void (async () => {
try { try {
const { buildOutputStyleTelemetry } = await import( const { buildOutputStyleTelemetry } =
"../services/compression/outputStyles/telemetry.ts" await import("../services/compression/outputStyles/telemetry.ts");
); const { insertCompressionRunTelemetryRow } =
const { insertCompressionRunTelemetryRow } = await import( await import("../../src/lib/db/compressionRunTelemetry.ts");
"../../src/lib/db/compressionRunTelemetry.ts"
);
const record = buildOutputStyleTelemetry({ const record = buildOutputStyleTelemetry({
requestId: skillRequestId ?? traceId ?? "", requestId: skillRequestId ?? traceId ?? "",
model: effectiveModel ?? "", model: effectiveModel ?? "",

View File

@@ -71,8 +71,11 @@ describe("Chat Pipeline — handleSingleModelChat decomposition", () => {
}); });
it("chatCore should record cost for both non-streaming and streaming responses", () => { it("chatCore should record cost for both non-streaming and streaming responses", () => {
// Non-streaming cost is still recorded inline; streaming cost was extracted to
// the recordStreamingCost leaf (open-sse/handlers/chatCore/streamingCost.ts,
// #4790 / #3501), so chatCore now delegates streaming cost to it.
assert.match(coreSrc, /if \(apiKeyInfo\?\.id && estimatedCost > 0\)/); assert.match(coreSrc, /if \(apiKeyInfo\?\.id && estimatedCost > 0\)/);
assert.match(coreSrc, /if \(apiKeyInfo\?\.id && streamUsage\)/); assert.match(coreSrc, /recordStreamingCost\(/);
}); });
}); });
@@ -208,12 +211,22 @@ describe("Plugin Architecture — plugins/hooks.ts", () => {
it("should run onRequest hooks in priority order", async () => { it("should run onRequest hooks in priority order", async () => {
const order = []; const order = [];
hooks.registerHook("onRequest", "first", () => { hooks.registerHook(
order.push("first"); "onRequest",
}, 1); "first",
hooks.registerHook("onRequest", "second", () => { () => {
order.push("second"); order.push("first");
}, 2); },
1
);
hooks.registerHook(
"onRequest",
"second",
() => {
order.push("second");
},
2
);
const ctx = { requestId: "r1", body: {}, model: "test", metadata: {} }; const ctx = { requestId: "r1", body: {}, model: "test", metadata: {} };
await hooks.runOnRequest(ctx); await hooks.runOnRequest(ctx);
@@ -221,13 +234,23 @@ describe("Plugin Architecture — plugins/hooks.ts", () => {
}); });
it("should support request blocking via emitHookBlocking", async () => { it("should support request blocking via emitHookBlocking", async () => {
hooks.registerHook("onRequest", "blocker", () => ({ hooks.registerHook(
blocked: true, "onRequest",
response: { error: "denied" }, "blocker",
}), 1); () => ({
hooks.registerHook("onRequest", "never-runs", () => { blocked: true,
throw new Error("should not run"); response: { error: "denied" },
}, 2); }),
1
);
hooks.registerHook(
"onRequest",
"never-runs",
() => {
throw new Error("should not run");
},
2
);
const ctx = { requestId: "r2", body: {}, model: "test", metadata: {} }; const ctx = { requestId: "r2", body: {}, model: "test", metadata: {} };
const result = await hooks.emitHookBlocking("onRequest", ctx); const result = await hooks.emitHookBlocking("onRequest", ctx);

View File

@@ -5,7 +5,8 @@ import path from "node:path";
import os from "node:os"; import os from "node:os";
// Dynamic import to pick up ESM module // Dynamic import to pick up ESM module
const { parseOpenapi, getEndpointsForArea } = await import("../../src/lib/agentSkills/openapiParser.ts"); const { parseOpenapi, getEndpointsForArea } =
await import("../../src/lib/agentSkills/openapiParser.ts");
// ─── Fixture helpers ────────────────────────────────────────────────────────── // ─── Fixture helpers ──────────────────────────────────────────────────────────
@@ -15,7 +16,9 @@ const { parseOpenapi, getEndpointsForArea } = await import("../../src/lib/agentS
*/ */
function withFixtureOpenapi(yamlContent: string): { cleanup: () => void } { function withFixtureOpenapi(yamlContent: string): { cleanup: () => void } {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-openapi-test-")); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-openapi-test-"));
const docsDir = path.join(tmpDir, "docs", "reference"); // openapiParser reads docs/openapi.yaml (consolidated location since #4781,
// previously docs/reference/openapi.yaml) — the fixture must mirror that path.
const docsDir = path.join(tmpDir, "docs");
fs.mkdirSync(docsDir, { recursive: true }); fs.mkdirSync(docsDir, { recursive: true });
fs.writeFileSync(path.join(docsDir, "openapi.yaml"), yamlContent, "utf-8"); fs.writeFileSync(path.join(docsDir, "openapi.yaml"), yamlContent, "utf-8");
@@ -118,7 +121,7 @@ test("parseOpenapi() groups /api/providers/* under 'providers' area", () => {
assert.ok(providerOps, "Expected 'providers' area to exist"); assert.ok(providerOps, "Expected 'providers' area to exist");
assert.ok( assert.ok(
providerOps!.length >= 5, providerOps!.length >= 5,
`Expected at least 5 provider endpoints, got ${providerOps!.length}`, `Expected at least 5 provider endpoints, got ${providerOps!.length}`
); );
const paths = providerOps!.map((op) => op.path); const paths = providerOps!.map((op) => op.path);
@@ -150,11 +153,11 @@ test("parseOpenapi() groups /api/v1/* under 'inference' area", () => {
assert.ok(inferenceOps, "Expected 'inference' area to exist"); assert.ok(inferenceOps, "Expected 'inference' area to exist");
assert.ok( assert.ok(
inferenceOps!.length >= 1, inferenceOps!.length >= 1,
`Expected at least 1 inference endpoint, got ${inferenceOps!.length}`, `Expected at least 1 inference endpoint, got ${inferenceOps!.length}`
); );
assert.ok( assert.ok(
inferenceOps!.some((op) => op.path === "/api/v1/chat/completions"), inferenceOps!.some((op) => op.path === "/api/v1/chat/completions"),
"Expected /api/v1/chat/completions in inference area", "Expected /api/v1/chat/completions in inference area"
); );
} finally { } finally {
cleanup(); cleanup();
@@ -184,7 +187,7 @@ test("parseOpenapi() throws if openapi.yaml is missing", () => {
assert.throws( assert.throws(
() => parseOpenapi(), () => parseOpenapi(),
/openapiParser: could not read/, /openapiParser: could not read/,
"Expected error when openapi.yaml is missing", "Expected error when openapi.yaml is missing"
); );
} finally { } finally {
process.chdir(originalCwd); process.chdir(originalCwd);
@@ -225,9 +228,9 @@ test(
assert.ok(providerOps, "Expected 'providers' area in real OpenAPI spec"); assert.ok(providerOps, "Expected 'providers' area in real OpenAPI spec");
assert.ok( assert.ok(
providerOps!.length >= 5, providerOps!.length >= 5,
`Expected ≥5 provider endpoints in real spec, got ${providerOps!.length}`, `Expected ≥5 provider endpoints in real spec, got ${providerOps!.length}`
); );
}, }
); );
test( test(
@@ -237,11 +240,11 @@ test(
const endpoints = getEndpointsForArea("providers"); const endpoints = getEndpointsForArea("providers");
assert.ok( assert.ok(
endpoints.length >= 5, endpoints.length >= 5,
`Expected ≥5 provider endpoint strings, got ${endpoints.length}: ${endpoints.join(", ")}`, `Expected ≥5 provider endpoint strings, got ${endpoints.length}: ${endpoints.join(", ")}`
); );
// Each entry should match "METHOD /path" // Each entry should match "METHOD /path"
for (const ep of endpoints) { for (const ep of endpoints) {
assert.match(ep, /^[A-Z]+ \//, `Endpoint "${ep}" does not match METHOD /path format`); assert.match(ep, /^[A-Z]+ \//, `Endpoint "${ep}" does not match METHOD /path format`);
} }
}, }
); );

View File

@@ -10,7 +10,11 @@
* *
* #4517. * #4517.
*/ */
import { describe, it, before, after, beforeEach } from "node:test"; // NOTE: tests/unit/autoCombo/ is a vitest-only scope (the node:test runner in
// test:unit:ci does not walk this dir), so this suite uses the vitest API even
// though it asserts via node:assert. (#4753 originally landed it with node:test
// imports → vitest reported "No test suite found" and the test ran nowhere.)
import { describe, it, beforeAll, afterAll, beforeEach } from "vitest";
import assert from "node:assert/strict"; import assert from "node:assert/strict";
import { import {
buildAutoCandidateFilter, buildAutoCandidateFilter,
@@ -20,7 +24,7 @@ import {
describe("suffixComposition :free tier (#4517)", () => { describe("suffixComposition :free tier (#4517)", () => {
const ORIGINAL_ENV = { ...process.env }; const ORIGINAL_ENV = { ...process.env };
before(() => { beforeAll(() => {
// Snapshot env so we can restore it after each test. // Snapshot env so we can restore it after each test.
}); });
@@ -30,7 +34,7 @@ describe("suffixComposition :free tier (#4517)", () => {
process.env = { ...ORIGINAL_ENV }; process.env = { ...ORIGINAL_ENV };
}); });
after(() => { afterAll(() => {
process.env = ORIGINAL_ENV; process.env = ORIGINAL_ENV;
}); });

View File

@@ -219,7 +219,11 @@ test("invalid cooldownByKind values (NaN, Infinity, negative) fall back to reset
cb.reset(); cb.reset();
cb._onFailure(kind); cb._onFailure(kind);
const t = cb._timeUntilReset(); const t = cb._timeUntilReset();
assert.ok(t > 29_000 && t <= 30_000, `expected resetTimeout fallback for ${kind}, got ${t}`); // _timeUntilReset() = resetTimeout - elapsed-since-failure, so the lower bound
// must tolerate real wall-clock drift on slow/loaded CI runners (this loop took
// ~1.6s once → t=28401, flaking the old `> 29_000`). Any t well above the invalid
// cooldown values (NaN/Infinity/negative) and <= resetTimeout proves the fallback.
assert.ok(t > 25_000 && t <= 30_000, `expected resetTimeout fallback for ${kind}, got ${t}`);
} }
cb.reset(); cb.reset();
}); });