fix(ci): drain the electron packaging regression, the models-catalog e2e assertion and 10 integration reds

Electron Package Smoke — a packaging defect that had been hidden behind another
packaging defect for nine days. Once the loginHeaderCapture fix let the main process
start, the server underneath died on 'Cannot find module next': resources/app/server.js
shipped without resources/app/node_modules.

electron-builder discards the ROOT node_modules in code, not by configuration —
app-builder-lib/out/util/filter.js:42 has a hard-coded `if (relative === "node_modules")
return false` that runs before any filter pattern. The second extraResources entry
pointing INTO ../.build/electron-standalone/node_modules is what sidesteps it, because
those relative paths are never equal to "node_modules". #10325 removed that entry as an
apparent duplicate and flipped the test to assert "exactly once", freezing the
regression as if it were the contract. Restored, and the unit guard now pins both
entries — proven by mutation: reverting package.json to the post-#10325 shape fails the
guard 3/4, restoring it passes 4/4.

group-b-quota-plans-config — the assertion was impossible to satisfy on ANY route, and
the page was never broken. layout.tsx hands the whole message catalogue to
NextIntlClientProvider, React serialises that prop into the RSC payload, and en.json
carries "Internal Server Error" twice, so page.content() always contains it: probing
/dashboard, /dashboard/costs, /dashboard/settings and /login showed the string present
with every page rendering fine, and a pageerror probe on the failing run captured zero
client exceptions. This is the same trap that killed the sibling not.toContain("500")
in fc77100c3f ("raw HTML is unreliable") — that one was removed, this one was kept.
Now asserts on rendered text, which still catches a real error boundary. The pageerror
capture stays: the CI failure carried no stack trace, which is why it was misread twice.

Integration — 10 of the 14 shard-2 reds, all sibling-test gaps behind security fixes:
monitoring health now takes a Request and requires management auth (GHSA-mvf8-qc78-5mxm);
the OAuth import routes moved to requireManagementAuth (GHSA-mg76) — the test accepts
both guard shapes and gained a stronger anchor that every exported handler awaits a
guard on its own request, mutation-verified; skill tool names are derived from
encodeSkillToolName() and the fake upstream now returns the encoded name so
decodeSkillToolName() is exercised too; previous_response_id now fails closed (#10262);
proxy_logs persist as an async batch (#11182) so the test flushes first;
providerQuotaOverrides joined GET /api/resilience (#9871); the reasoning fixture used a
model that stopped being thinking-incompatible, replaced and pinned with a premise
assert so it cannot rot silently again.

A vacuous assert.ok(true, "all 10 streams completed without hanging") was replaced with
real anchors — content must arrive on every stream and the active Timeout count must not
grow.

Four are deliberately left red rather than aligned, each now tracked: #11551 (the
/v1/models after() wiring is dead — the route passes a third argument to a two-parameter
function and catalogCache never imports after, so the #8728 contract is unimplemented),
#11552 (~27% of requests emit an extra discarded upstream call; the delivered
distribution is exactly 0.70, so weighted routing is correct and the waste is the real
finding), the fixed-account combo pin (aligning it would destroy the per-step attribution
the test exists for), and the web_search fallback already tracked as #11524.

Package Artifact — the provenance stamp I added last round used git rev-parse HEAD, which
under pull_request is the ephemeral merge commit and therefore never an ancestor of the
release branch. Now takes the PR head sha.

Refs #10692
This commit is contained in:
Xiangzhe
2026-08-25 16:46:01 -03:00
parent 7790b0d168
commit c51d74213e
11 changed files with 235 additions and 38 deletions

View File

@@ -692,7 +692,17 @@ jobs:
# vouchear). Sem este passo o par build+validate deste job e estruturalmente
# incompativel e falha 100% das vezes.
- name: Stamp dist/BUILD_SHA for the provenance guard (#10427)
run: node scripts/build/write-build-sha.mjs
# O SHA TEM de vir do head da PR, nao de `git rev-parse HEAD`. Este workflow
# roda em `pull_request`, entao o checkout e o MERGE COMMIT efemero que o
# GitHub cria — um commit que nao existe em branch nenhuma e portanto nunca e
# ancestral da release. O guard de proveniencia (#10427) rejeita exatamente
# isso, e com razao: um artefato carimbado com o merge commit nao pode ser
# rastreado ate codigo que passou pelos gates.
env:
OMNIROUTE_BUILD_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
run: |
export OMNIROUTE_BUILD_SHA="${OMNIROUTE_BUILD_SHA:0:7}"
node scripts/build/write-build-sha.mjs
- run: npm run check:pack-artifact
# WS1.2 (#7065 class): pack the real tarball, install it into a clean prefix and
# BOOT it to a healthy /api/monitoring/health — the gate that structure checks

View File

@@ -78,6 +78,14 @@
{
"from": "../.build/electron-standalone",
"to": "app",
"filter": [
"**/*",
"node_modules/**/*"
]
},
{
"from": "../.build/electron-standalone/node_modules",
"to": "app/node_modules",
"filter": [
"**/*"
]

View File

@@ -14,7 +14,20 @@ import { test, expect } from "@playwright/test";
import { gotoDashboardRoute } from "./helpers/dashboardAuth";
test.describe("Group B — Quota Plans Config", () => {
// Client-side exception capture. Without it a page that falls into the error
// boundary only shows up as "Internal Server Error" in the HTML, with no stack
// trace anywhere in the CI log — which is exactly how this spec's failure went
// undiagnosed for two CI rounds.
const pageErrors: string[] = [];
test.beforeEach(async ({ page }) => {
pageErrors.length = 0;
page.on("pageerror", (err) => {
pageErrors.push(`[pageerror] ${err.message}\n${err.stack ?? ""}`);
});
page.on("console", (msg) => {
if (msg.type() === "error") pageErrors.push(`[console.error] ${msg.text()}`);
});
// Mock the plans list endpoint
await page.route("**/api/quota/plans**", async (route) => {
const url = new URL(route.request().url());
@@ -143,12 +156,25 @@ test.describe("Group B — Quota Plans Config", () => {
}
// After selection, the page should not be in a broken state.
// Note: page.content() includes the full HTML source, which contains Next.js
// chunk filenames — those hashes can legitimately contain the string "500".
// Checking for "500" in raw HTML is unreliable; instead check for the actual
// error boundary text that OmniRoute renders on unrecoverable errors
// (src/app/error.tsx heading: "Internal Server Error").
const pageContent = await page.content();
expect(pageContent).not.toContain("Internal Server Error");
//
// Assert on RENDERED TEXT, not on page.content(). The raw HTML always contains
// the string, on every route, so the old assertion could never pass: layout.tsx
// hands the whole message catalogue to NextIntlClientProvider, React serialises
// that prop into the RSC payload, and en.json carries "Internal Server Error"
// twice (publicSystem.error.title and errors.500.title). Probing /dashboard,
// /dashboard/costs, /dashboard/settings and even /login all showed the string
// present in the source with the page rendering perfectly.
//
// This is the same trap that killed the sibling `not.toContain("500")` here in
// fc77100c3f ("Checking for '500' in raw HTML is unreliable") — that one was
// removed, this one was kept, and it has the identical flaw.
//
// The error boundary renders the title as visible text (src/app/error.tsx
// `<h1>{t("error.title")}</h1>`), so innerText still catches the real defect
// while ignoring the serialised dictionary.
const bodyText = await page.locator("body").innerText();
expect(bodyText, `client errors:\n${pageErrors.join("\n---\n")}`).not.toContain(
"Internal Server Error"
);
});
});

View File

@@ -19,6 +19,7 @@ const { getLatestCallLog, getResponsesCallLogs } = await import("./_chatPipeline
const { invalidateMemorySettingsCache } = await import("../../src/lib/memory/settings.ts");
const { skillRegistry } = await import("../../src/lib/skills/registry.ts");
const { skillExecutor } = await import("../../src/lib/skills/executor.ts");
const { encodeSkillToolName } = await import("../../src/lib/skills/injection.ts");
const { handleChat } = await import("../../src/sse/handlers/chat.ts");
const { initTranslators } = await import("../../open-sse/translator/index.ts");
const { clearInflight } = await import("../../open-sse/services/requestDedup.ts");
@@ -726,7 +727,7 @@ test("chat pipeline applies Codex CLI fingerprint to OAuth responses requests",
);
});
test("chat pipeline strips previous_response_id from stateless Codex responses by default", async () => {
test("chat pipeline fails closed on an unresolvable previous_response_id and keeps stateless Codex responses stateless", async () => {
await seedConnection("codex", {
apiKey: "sk-codex-stateless-responses",
providerSpecificData: { openaiStoreEnabled: false },
@@ -760,9 +761,38 @@ test("chat pipeline strips previous_response_id from stateless Codex responses b
})
);
await response.json();
// #10262 virtualized `previous_response_id`: in any mode other than "preserve"
// the id is resolved against OmniRoute's own continuation store BEFORE routing.
// An id it cannot resolve fails closed with OpenAI's own contract instead of
// being silently stripped and forwarded as a fresh turn (which would have
// dropped the conversation history without telling the client).
const failClosed = (await response.json()) as { error?: { code?: string } };
assert.equal(response.status, 400);
assert.equal(failClosed.error?.code, "previous_response_not_found");
assert.equal(fetchCalls.length, 0, "a request that fails closed must not reach the upstream");
assert.equal(response.status, 200);
// Positive anchor: the same stateless Codex connection, without the unresolvable
// continuation id, still dispatches — and the stateless contract still holds
// (store:false, no previous_response_id on the wire).
const followUp = await handleChat(
buildRequest({
url: "http://localhost/v1/responses",
body: {
model: "codex/gpt-5.5",
stream: false,
input: [
{
type: "message",
role: "user",
content: [{ type: "input_text", text: "First VS Code turn" }],
},
],
},
})
);
await followUp.json();
assert.equal(followUp.status, 200);
assert.equal(fetchCalls.length, 1);
assert.match(fetchCalls[0].url, /\/responses$/);
assert.equal(fetchCalls[0].body.previous_response_id, undefined);
@@ -1440,13 +1470,23 @@ test("chat pipeline injects skills into tools and intercepts tool calls with ski
enabled: true,
});
// #9058: provider tool names must match ^[a-zA-Z0-9_-]+$, so `name@version`
// identifiers travel base64url-encoded. Derive the expectation from the helper
// instead of pinning the encoded literal.
const expectedSkillToolName = encodeSkillToolName("lookupWeather", "1.0.0");
assert.match(expectedSkillToolName, /^[a-zA-Z0-9_-]+$/);
assert.notEqual(expectedSkillToolName, "lookupWeather@1.0.0");
const fetchCalls = [];
globalThis.fetch = async (url, init: RequestInit = {}) => {
fetchCalls.push({
url: String(url),
body: init.body ? JSON.parse(String(init.body)) : null,
});
return buildOpenAIToolCallResponse();
// #9058: the upstream echoes back exactly the tool name it was given — the
// provider-safe encoded one — so this also exercises decodeSkillToolName()
// on the interception path.
return buildOpenAIToolCallResponse({ toolName: expectedSkillToolName });
};
const response = await handleChat(
@@ -1464,7 +1504,7 @@ test("chat pipeline injects skills into tools and intercepts tool calls with ski
assert.equal(response.status, 200);
assert.equal(fetchCalls.length, 1);
assert.ok(Array.isArray(fetchCalls[0].body.tools));
assert.equal(fetchCalls[0].body.tools[0].function.name, "lookupWeather@1.0.0");
assert.equal(fetchCalls[0].body.tools[0].function.name, expectedSkillToolName);
assert.equal(json.choices[0].finish_reason, "tool_calls");
assert.equal(json.tool_results[0].tool_call_id, "call_weather");
assert.equal(JSON.parse(json.tool_results[0].output).forecast, "Sunny in Sao Paulo");

View File

@@ -22,8 +22,25 @@ process.env.JWT_SECRET = "test-health-cache-secret";
await import("../../src/lib/db/core.ts");
const { GET, DELETE } = await import("../../src/app/api/monitoring/health/route.ts");
// GHSA-mvf8-qc78-5mxm: the detailed health payload (the one carrying `timestamp`)
// is reserved for a management principal — GET now takes the Request and an
// anonymous caller only gets the liveness verdict. Every probe below therefore
// authenticates with a dashboard-session cookie, exactly like the DELETE probe.
const { SignJWT } = await import("jose");
const AUTH_TOKEN = await new SignJWT({ authenticated: true })
.setProtectedHeader({ alg: "HS256" })
.setExpirationTime("30d")
.sign(new TextEncoder().encode(process.env.JWT_SECRET as string));
function authedRequest(method = "GET"): Request {
return new Request("http://localhost/api/monitoring/health", {
method,
headers: { cookie: `auth_token=${AUTH_TOKEN}` },
});
}
async function healthTimestamp(): Promise<string> {
const res = await GET();
const res = await GET(authedRequest());
const body = (await res.json()) as {
timestamp?: string;
adaptiveAdmission?: unknown;
@@ -48,19 +65,8 @@ test("cache expires after the TTL — a fresh payload is built", async () => {
});
test("DELETE (circuit-breaker reset) invalidates the cache immediately", async () => {
const { SignJWT } = await import("jose");
const authToken = await new SignJWT({ authenticated: true })
.setProtectedHeader({ alg: "HS256" })
.setExpirationTime("30d")
.sign(new TextEncoder().encode(process.env.JWT_SECRET as string));
const t1 = await healthTimestamp(); // populate cache
const delRes = await DELETE(
new Request("http://localhost/api/monitoring/health", {
method: "DELETE",
headers: { cookie: `auth_token=${authToken}` },
})
);
const delRes = await DELETE(authedRequest("DELETE"));
assert.ok(delRes.status < 400, `DELETE should succeed, got ${delRes.status}`);
await new Promise((r) => setTimeout(r, 5)); // ensure the clock advances past ms precision
const t2 = await healthTimestamp();

View File

@@ -176,6 +176,11 @@ test("integration: proxy registry full flow works and enforces safe delete", asy
provider: "openai",
});
// #11182 made SQLite persistence a background batch (1s timer / 100-entry
// threshold); the health aggregate reads the table, so drain the queue first
// instead of racing the timer.
proxyLogger.flushProxyLogsSync();
const healthRes = await proxyHealthRoute.GET(
new Request("http://localhost/api/settings/proxies/health?hours=24")
);

View File

@@ -3,6 +3,10 @@ import assert from "node:assert/strict";
import { createChatPipelineHarness } from "./_chatPipelineHarness.ts";
const harness = await createChatPipelineHarness("reasoning-routing-pipeline");
// Imported only AFTER the harness has set DATA_DIR and opened the DB: a static
// import is evaluated before any module body runs, and this module touches the
// settings/DB layer at load time.
const { getResolvedModelCapabilities } = await import("../../src/lib/modelCapabilities.ts");
const {
BaseExecutor,
buildOpenAIResponse,
@@ -346,10 +350,20 @@ test("reasoning routing filters incompatible combo targets and rejects an empty
await reasoningRulesDb.deleteReasoningRoutingRule(
(await reasoningRulesDb.getReasoningRoutingRules())[0].id
);
// The target must be a model that DECLARES it cannot think (supportsThinking:
// false). `unknown` capability is deliberately kept by
// filterComboForReasoningDecision, so a model whose thinking support later
// becomes true/unknown silently stops exercising this path — assert the premise.
const INCOMPATIBLE_TARGET = "openai/gpt-4o";
assert.equal(
getResolvedModelCapabilities(INCOMPATIBLE_TARGET).supportsThinking,
false,
`${INCOMPATIBLE_TARGET} must declare supportsThinking:false for this test to mean anything`
);
const incompatibleCombo = await combosDb.createCombo({
name: "incompatible-reasoning-combo",
strategy: "priority",
models: ["antigravity/gemini-3-pro"],
models: [INCOMPATIBLE_TARGET],
});
await reasoningRulesDb.createReasoningRoutingRule({
name: "Empty combo target",

View File

@@ -555,12 +555,15 @@ test("resilience API only exposes configuration, not runtime breaker state", asy
const { response, json } = await getJson(`${app.baseUrl}/api/resilience`);
assert.equal(response.status, 200);
// Exact key set — this is the whole point of the test: configuration only.
// `providerQuotaOverrides` joined the projection in #9871.
assert.deepEqual(Object.keys(json).sort(), [
"comboCooldownWait",
"connectionCooldown",
"legacy",
"providerBreaker",
"providerCooldown",
"providerQuotaOverrides",
"quotaShareConcurrencyLimit",
"requestQueue",
"waitForCooldown",

View File

@@ -303,8 +303,40 @@ test("OAuth routes that can create provider connections require auth guard", ()
for (const relPath of targets) {
const content = readIfExists(relPath);
assert.ok(content, `${relPath} should exist`);
assert.ok(content.includes("isAuthRequired"), `${relPath} should check whether auth is active`);
assert.ok(content.includes("isAuthenticated"), `${relPath} should require authenticated users`);
assert.ok(content.includes("Unauthorized"), `${relPath} should reject anonymous requests`);
// Two accepted guard shapes. GHSA-mg76 moved the cursor/kiro *import* routes
// onto requireManagementAuth, which is strictly STRONGER than the legacy
// pair: it demands a management principal (dashboard session, manage-scoped
// key, CLI token) instead of merely "any authenticated caller", and answers
// 401/403 itself — so the literal "Unauthorized" no longer appears in the
// route file. The remaining routes still carry the legacy triple.
const usesManagementGuard = content.includes("requireManagementAuth(request");
const usesLegacyGuard =
content.includes("isAuthRequired") &&
content.includes("isAuthenticated") &&
content.includes("Unauthorized");
assert.ok(
usesManagementGuard || usesLegacyGuard,
`${relPath} must guard connection-creating handlers with requireManagementAuth or the isAuthRequired/isAuthenticated pair`
);
// Positive anchor: a guard somewhere in the file proves nothing if one of the
// exported handlers skips it. Slice the file per exported handler and require
// EACH body to await a guard on its own `request` — a guard living only in a
// helper (or in a sibling handler) no longer satisfies this.
const handlerSlices = content
.split(/(?=export\s+async\s+function\s+(?:GET|POST|PUT|PATCH|DELETE)\b)/)
.filter((slice) =>
/^export\s+async\s+function\s+(?:GET|POST|PUT|PATCH|DELETE)\b/.test(slice)
);
assert.ok(handlerSlices.length > 0, `${relPath} should export at least one HTTP handler`);
for (const slice of handlerSlices) {
const verb = /export\s+async\s+function\s+(\w+)/.exec(slice)?.[1];
assert.match(
slice,
/await\s+(?:require\w*Auth|isAuthRequired)\s*\(\s*(?:request|req)\b/,
`${relPath}: exported handler ${verb} does not await an auth guard on its own request`
);
}
}
});

View File

@@ -76,17 +76,43 @@ test("2. client cancel propagates to upstream (abort propagation)", async () =>
test("3. no leaked idle timers across N sequential streams", async () => {
// createSSEStream installs a setInterval idle watchdog per stream.
// If cleanup (clearInterval) does not run on stream close, timers accumulate.
// This test creates 10 streams and drains them; it acts as a smoke test that
// the process does not hang (a leaked setInterval that fires 10s later would
// prevent the test process from exiting cleanly in --test-force-exit mode).
for (let i = 0; i < 10; i++) {
// Each stream carries a real content delta: a stream whose upstream forwards
// no valuable chunk is rejected by the empty-content guard
// (open-sse/utils/streamEmptyChoices.ts) and would never reach the flush path
// whose cleanup this test is about.
//
// Drained inline, without drain()'s timeout guard: that guard leaves its own
// uncleared setTimeout behind and would drown out the very signal measured here.
const activeTimers = () =>
process.getActiveResourcesInfo().filter((resource) => resource === "Timeout").length;
const timersBefore = activeTimers();
const N = 10;
for (let i = 0; i < N; i++) {
const { up, out } = makeStream();
up.push(`data: {"choices":[{"delta":{"content":"chunk-${i}"}}]}\n\n`);
up.push("data: [DONE]\n\n");
up.close();
await drain(out);
const reader = out.getReader();
const decoder = new TextDecoder();
let text = "";
for (;;) {
const { done, value } = await reader.read();
if (done) break;
text += decoder.decode(value);
}
// Positive anchor: the stream really ran and really closed.
assert.ok(text.includes(`chunk-${i}`), `stream ${i} lost its content: ${JSON.stringify(text)}`);
}
// If we reach here without a timeout, no blocking resources were leaked.
assert.ok(true, "all 10 streams completed without hanging");
// The watchdog of every closed stream must have been cleared. One slot of slack
// absorbs unrelated runtime timers, but N leaked watchdogs cannot hide in it.
const timersAfter = activeTimers();
assert.ok(
timersAfter <= timersBefore + 1,
`idle watchdog timers leaked across ${N} streams: ${timersBefore} active before, ${timersAfter} after`
);
});
test("4. final snapshot does not duplicate tail text", async () => {

View File

@@ -7,7 +7,29 @@ import { pruneElectronRuntimeDocs } from "../../scripts/build/electronRuntimeDoc
const ROOT = join(import.meta.dirname, "..", "..");
test("electron build copies the standalone runtime into resources/app exactly once", () => {
// The SECOND entry looks like a redundant duplicate of the first — it is not, and
// removing it ships a desktop app that cannot boot.
//
// electron-builder's file matcher hard-codes an exclusion of the source root's
// `node_modules` directory for extraResources/extraFiles, BEFORE any `filter`
// pattern is consulted (app-builder-lib/out/util/filter.js: `if (relative ===
// "node_modules") return false`). So `{ from: ".build/electron-standalone", to:
// "app", filter: ["**/*"] }` copies server.js, server-ws.mjs and every NESTED
// node_modules, but silently drops `.build/electron-standalone/node_modules` —
// the tree that holds `next`, `better-sqlite3` and the whole server closure.
//
// Pointing a second matcher AT the node_modules directory sidesteps the check
// (its relative paths never equal "node_modules") and is the only way to get that
// tree into `resources/app/node_modules`, which main.js also puts on the server's
// NODE_PATH.
//
// Regression history: #10325 "de-duplicated" the two entries into one on
// 2026-08-16; the packaged app then died on `Cannot find module 'next'` at
// resources/app/server.js. It went unnoticed for nine days because the Electron
// Package Smoke was already red on an earlier defect (lib/loginHeaderCapture.js
// missing from build.files since #9984), so the main process never got far enough
// to spawn the server.
test("electron build copies the standalone runtime AND its root node_modules into resources/app", () => {
const electronPackage = JSON.parse(readFileSync(join(ROOT, "electron", "package.json"), "utf8"));
const extraResources = electronPackage.build?.extraResources;
@@ -21,6 +43,11 @@ test("electron build copies the standalone runtime into resources/app exactly on
{
from: "../.build/electron-standalone",
to: "app",
filter: ["**/*", "node_modules/**/*"],
},
{
from: "../.build/electron-standalone/node_modules",
to: "app/node_modules",
filter: ["**/*"],
},
]);