From 5eccbfac19250d323beec93d270aa6e37d853de2 Mon Sep 17 00:00:00 2001 From: Xiangzhe Date: Tue, 25 Aug 2026 09:24:40 -0300 Subject: [PATCH] fix(ci): drain four inherited base-reds in packaging, electron and integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four predate this session — each reproduces identically on f95b03d70 (2026-08-24), so none is a cycle regression. Draining them here because the release pre-flight is where inherited reds get resolved. 1. Package Artifact: the job runs `build:cli`, which assembles dist/ but never writes dist/BUILD_SHA — only `build:release` does, via write-build-sha.mjs. The #10427 provenance guard inside check:pack-artifact then rejects the artifact as untraceable, and rejects it even under OMNIROUTE_ALLOW_CANARY_BUILD. The job's build+validate pair was structurally incompatible and failed 100% of the time. Stamps the SHA between the two steps. 2. Electron Package Smoke: electron/package.json's build.files allowlist enumerates each lib/*.js by hand and never got lib/loginHeaderCapture.js, added alongside its require() in #9984. The file therefore stayed out of app.asar and the packaged app died at startup on 'Cannot find module ./lib/loginHeaderCapture'. 3. proxy-pipeline: the breaker assertion grepped chat.ts for executeChatWithBreaker(, but that call moved behind the chatDispatch.ts seam. Rather than drop the check, it now pins both hops — chat.ts dispatches through the seam and the seam calls the breaker — so the extraction cannot silently take the breaker off the path. 4. skills-pipeline: #9058 began encoding skill tool names as omr_skill_ because providers require ^[a-zA-Z0-9_-]+$, and these assertions still expected the raw name@version. They now derive the expected name from encodeSkillToolName(), the same helper production uses, so the test tracks the contract instead of duplicating it. Only the assertions about names on the wire were converted; the identifiers passed straight to skillExecutor.execute() stay raw, because those are not encoded. Integration suite for these two files: 54/55. The one still red — 'web_search fallback preserves Responses API output' — is a separate pre-existing defect, deliberately left failing rather than papered over: on the /v1/responses path resolveSearchCredentials() returns null for the seeded serper-search connection, so executeWebSearch.ts:185-200 falls through to the cheapest fallbackOnly provider (duckduckgo-free) and the results come back empty. The sibling chat-path test seeds identically and does resolve serper-search. Needs its own investigation. Refs #10692 --- .github/workflows/ci.yml | 8 +++++++ electron/package.json | 1 + tests/integration/proxy-pipeline.test.ts | 12 ++++++++-- tests/integration/skills-pipeline.test.ts | 28 +++++++++++++++++------ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4491f10df1..7e315fbf72 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -685,6 +685,14 @@ jobs: - run: npm run build:cli - name: Assert dist/server.js exists run: test -f dist/server.js || (echo "dist/server.js missing — build:cli did not assemble correctly" && exit 1) + # `build:cli` monta dist/ mas NAO grava dist/BUILD_SHA — so `build:release` faz + # isso, chamando write-build-sha.mjs. O guard de proveniencia do #10427, dentro + # de check:pack-artifact, rejeita um artefato sem SHA (e rejeita mesmo com + # OMNIROUTE_ALLOW_CANARY_BUILD=1: o que nao da para identificar nao da para + # 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 - 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 diff --git a/electron/package.json b/electron/package.json index bdb8b205f5..63a97d527c 100644 --- a/electron/package.json +++ b/electron/package.json @@ -65,6 +65,7 @@ "lib/resolveServerEntry.js", "lib/resolveNodeHelper.js", "lib/windowLifecycle.js", + "lib/loginHeaderCapture.js", "lib/resolveRemoteServerUrl.js", "lib/remoteServerPreferences.js", "lib/serverReadiness.js", diff --git a/tests/integration/proxy-pipeline.test.ts b/tests/integration/proxy-pipeline.test.ts index 48e1536202..ba1d3542a8 100644 --- a/tests/integration/proxy-pipeline.test.ts +++ b/tests/integration/proxy-pipeline.test.ts @@ -37,6 +37,7 @@ describe("Chat Pipeline — handleSingleModelChat decomposition", () => { const src = readSrc("sse/handlers/chat.ts"); const helpersSrc = readSrc("sse/handlers/chatHelpers.ts"); const coreSrc = readOpenSse("handlers/chatCore.ts"); + const dispatchSrc = readSrc("sse/handlers/chatDispatch.ts"); it("should define resolveModelOrError helper", () => { assert.ok(helpersSrc, "chatHelpers.ts should exist"); @@ -66,8 +67,15 @@ describe("Chat Pipeline — handleSingleModelChat decomposition", () => { assert.match(src, /checkPipelineGates\(provider/); }); - it("handleSingleModelChat should use executeChatWithBreaker", () => { - assert.match(src, /executeChatWithBreaker\(/); + // O breaker deixou de ser chamado direto por handleSingleModelChat: a chamada foi + // extraida para o seam chatDispatch.ts (dispatchChatWithAffinityEviction). O + // invariante que este teste protege continua o mesmo — todo dispatch de chat passa + // pelo circuit breaker — mas agora precisa ser verificado nos DOIS saltos, senao a + // extracao poderia remover o breaker do caminho sem nenhum teste reclamar. + it("handleSingleModelChat should dispatch through the breaker seam", () => { + assert.match(src, /dispatchChatWithAffinityEviction\(/); + assert.ok(dispatchSrc, "src/sse/handlers/chatDispatch.ts should exist"); + assert.match(dispatchSrc, /executeChatWithBreaker\(/); }); it("chatCore should record cost for both non-streaming and streaming responses", () => { diff --git a/tests/integration/skills-pipeline.test.ts b/tests/integration/skills-pipeline.test.ts index f953d62096..4d4fbd6698 100644 --- a/tests/integration/skills-pipeline.test.ts +++ b/tests/integration/skills-pipeline.test.ts @@ -1,6 +1,13 @@ import test from "node:test"; import assert from "node:assert/strict"; import { OMNIROUTE_WEB_SEARCH_FALLBACK_TOOL_NAME } from "../../open-sse/services/webSearchFallback.ts"; +// Skill identifiers sao name@version, que viola o ^[a-zA-Z0-9_-]+$ exigido por +// OpenAI/DeepSeek/Groq, entao injection.ts os codifica como omr_skill_ +// (#9058). Derivar o nome esperado do MESMO helper que a producao usa, em vez de +// repetir a string codificada, mantem o teste preso ao contrato: se a codificacao +// mudar de novo, o assert acompanha; se ela sumir, o assert continua exigindo que +// producao e teste concordem. +import { encodeSkillToolName } from "../../src/lib/skills/injection.ts"; import { createChatPipelineHarness } from "./_chatPipelineHarness.ts"; @@ -137,7 +144,11 @@ test("enabling a disabled skill makes it available in the request pipeline", asy assert.equal(updateResponse.status, 200); assert.equal(response.status, 200); assert.ok(Array.isArray(fetchBodies[0].tools)); - assert.ok(fetchBodies[0].tools.some((tool) => tool.function.name === "lookupWeather@1.0.0")); + assert.ok( + fetchBodies[0].tools.some( + (tool) => tool.function.name === encodeSkillToolName("lookupWeather", "1.0.0") + ) + ); }); test("matching tool calls execute the registered skill and return tool results", async () => { @@ -156,7 +167,7 @@ test("matching tool calls execute the registered skill and return tool results", globalThis.fetch = async () => buildOpenAIToolCallResponse({ - toolName: "lookupWeather@1.0.0", + toolName: encodeSkillToolName("lookupWeather", "1.0.0"), argumentsObject: { location: "Recife" }, }); @@ -351,7 +362,7 @@ test("injectSkills() correctly injects skill context into a request", async () = assert.ok(Array.isArray(tools), "injectSkills should return an array"); assert.equal(tools.length, 1, "should inject exactly one skill tool"); assert.equal((tools[0] as any).type, "function"); - assert.equal((tools as any)[0].function.name, "translateText@1.0.0"); + assert.equal((tools as any)[0].function.name, encodeSkillToolName("translateText", "1.0.0")); (assert as any).equal( (tools[0] as any).function.description, "Translate text to another language" @@ -388,7 +399,7 @@ test("injectSkills() merges with existing tools without duplicating", async () = assert.equal(tools.length, 2, "should have injected skill + existing tool"); const names = tools.map((t) => (t as any).function?.name || (t as any).name); - assert.ok(names.includes("calcRoute@1.0.0")); + assert.ok(names.includes(encodeSkillToolName("calcRoute", "1.0.0"))); assert.ok(names.includes("preExistingTool")); }); @@ -447,8 +458,8 @@ test("responses input context participates in AUTO skill injection", async () => .map((tool) => tool?.function?.name) .filter((name) => typeof name === "string"); - assert.ok(names.includes("issueSearch@1.0.0")); - assert.ok(!names.includes("calendarPlanner@1.0.0")); + assert.ok(names.includes(encodeSkillToolName("issueSearch", "1.0.0"))); + assert.ok(!names.includes(encodeSkillToolName("calendarPlanner", "1.0.0"))); }); test("handleToolCallExecution() processes a tool call correctly", async () => { @@ -778,7 +789,10 @@ test("builtin and custom skills coexist in the injected tool list", async () => const toolNames = (fetchBodies[0].tools || []).map((tool) => tool.function.name).sort(); assert.equal(response.status, 200); - assert.deepEqual(toolNames, ["lookupWeather@1.0.0", "webSearch@1.0.0"]); + assert.deepEqual(toolNames, [ + encodeSkillToolName("lookupWeather", "1.0.0"), + encodeSkillToolName("webSearch", "1.0.0"), + ]); }); test("web_search fallback converts built-in tools for unsupported providers and executes search", async () => {