fix(sse): scan Devin and PromptQL tool markup in linear time (#15099)

Replaces the cubic `<tool>\s*([\s\S]*?)\s*</tool>`, `<summary>` and `<final_response>` regexes with the shared linear findTagBlocks scanner (sibling of #15066).

Evidence (tip ed664fc8 merged into the head): new test hangs to the 400 s timeout on the tip and passes 6/6 in ~1 s with the fix; 18 Devin/PromptQL neighbor test files green; typecheck:core, check:open-sse-typecheck, eslint, prettier and check-file-size clean.

Thanks @HouMinXi!
This commit is contained in:
Bob.Hou 2026-09-29 12:29:16 -04:00 • committed by GitHub
parent 666ea59b4c
commit 1aa0af9cbf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 371 additions and 27 deletions

View file

@ -0,0 +1 @@
- **fix(ci):** `validate-release-green` parses `ci.yml` with the already-declared `js-yaml` dependency instead of the undeclared `yaml` package, so a main-green nightly crash no longer posts an empty verdict on the tracker.

View file

@ -0,0 +1 @@
- **fix(memory):** assemble the optional `@huggingface/transformers` specifier at runtime with `webpackIgnore`, so Next does not fail `/health` when the optional package is absent.

View file

@ -0,0 +1 @@
- Electron release: `electron/package-lock.json` regained the optional `electron-builder-squirrel-windows` subtree (13 entries) that `npm ci` had been refusing as out of sync — the Linux desktop leg died on it — and `electron-release.yml` gained a `build_ref` dispatch input so a release whose tag was cut with the broken lock can have its assets rebuilt from the repaired line

View file

@ -0,0 +1 @@
- Electron release workflow: the `publish-npm` job now grants `actions: read` to the reusable `npm-publish.yml` it calls (its `publish` job requests it), which is what made GitHub refuse the whole v3.8.50 run at startup and ship the release with zero desktop assets; a `workflow_dispatch` now builds the requested tag instead of the dispatching branch and can skip the npm leg (`publish_npm=false`) when only re-attaching assets

View file

@ -0,0 +1 @@
- npm publish workflow: the CycloneDX SBOM is attached to the GitHub Release on `workflow_dispatch` publishes too (when a release for the tag exists), not only on the `release` event — v3.8.50 shipped through a staged dispatch and its release carried no SBOM until it was attached by hand from the run's `sbom-npm` artifact

View file

@ -0,0 +1 @@
- `Coverage` job on `ci.yml`: the informational Codecov upload gets its own 5-minute ceiling and `continue-on-error`, and the job budget grows from 20 to 30 minutes (the 8-shard c8 merge alone takes ~10) — a stalled upload no longer ends the job `cancelled` and drags a fully green `main` run's conclusion down with it

View file

@ -1,5 +1,6 @@
import { createHash } from "node:crypto";
import { asRecord, DevinAgenticBridgeError, type AnthropicTool, type JsonRecord } from "./types.ts";
import { findTagBlocks } from "../../utils/tagBlocks.ts";
function stableJson(value: unknown): string {
if (Array.isArray(value)) return `[${value.map((item) => stableJson(item)).join(",")}]`;
@ -66,7 +67,8 @@ function validateSchema(value: unknown, schema: JsonRecord, path: string): strin
return errors;
}
const BARE_SUMMARY_ENVELOPE_RE = /^<summary>\s*([\s\S]*?)\s*<\/summary>$/i;
const SUMMARY_OPEN = "<summary>";
const SUMMARY_CLOSE = "</summary>";
/**
* Detects a whole-response Devin "summarizer" progress report: an internal
@ -77,12 +79,20 @@ const BARE_SUMMARY_ENVELOPE_RE = /^<summary>\s*([\s\S]*?)\s*<\/summary>$/i;
*/
export function extractBareSummaryEnvelope(text: string): string | null {
const trimmed = text.trim();
const match = trimmed.match(BARE_SUMMARY_ENVELOPE_RE);
return match ? match[1].trim() : null;
// Anchored prefix/suffix checks instead of one `^<summary>\s*([\s\S]*?)\s*<\/summary>$`
// pattern, whose overlapping whitespace classes are cubic on a long run of spaces.
if (trimmed.length < SUMMARY_OPEN.length + SUMMARY_CLOSE.length) return null;
if (!/^<summary>/i.test(trimmed) || !/<\/summary>$/i.test(trimmed)) return null;
return trimmed.slice(SUMMARY_OPEN.length, trimmed.length - SUMMARY_CLOSE.length).trim();
}
export function parseDevinToolRequest(text: string, tools: AnthropicTool[], idSeed = "") {
const matches = [...text.matchAll(/<tool>\s*([\s\S]*?)\s*<\/tool>/g)];
// `text` is model output; scan forward once instead of matching `<tool>\s*([\s\S]*?)\s*<\/tool>`,
// which is cubic on a long run of spaces after an unclosed tag.
const matches = findTagBlocks(text, /<tool>/g, /<\/tool>/g).map((block) => ({
whole: text.slice(block.start, block.end),
inner: block.inner.trim(),
}));
if (matches.length === 0) return null;
if (matches.length > 1) {
throw new DevinAgenticBridgeError(
@ -91,7 +101,7 @@ export function parseDevinToolRequest(text: string, tools: AnthropicTool[], idSe
);
}
if (text.trim() !== matches[0][0].trim()) {
if (text.trim() !== matches[0].whole.trim()) {
throw new DevinAgenticBridgeError(
"Devin tool request must be a standalone tool envelope without narrative text",
"mixed_tool_narrative"
@ -100,7 +110,7 @@ export function parseDevinToolRequest(text: string, tools: AnthropicTool[], idSe
let payload: JsonRecord;
try {
payload = asRecord(JSON.parse(matches[0][1] || "{}"));
payload = asRecord(JSON.parse(matches[0].inner || "{}"));
} catch {
throw new DevinAgenticBridgeError("Devin tool request was not valid JSON", "invalid_tool_json");
}

View file

@ -6,6 +6,8 @@
* embedded as `<final_response>…</final_response>` XML inside `response_text`).
*/
import { findTagBlocks } from "../../utils/tagBlocks.ts";
export function walkStrings(
node: unknown,
out: Array<{ path: string; text: string }> = [],
@ -13,11 +15,7 @@ export function walkStrings(
): Array<{ path: string; text: string }> {
if (node == null) return out;
if (typeof node === "string") {
if (
node.length >= 1 &&
!/^[0-9a-f-]{36}$/i.test(node) &&
!/^\d{4}-\d{2}-\d{2}T/.test(node)
) {
if (node.length >= 1 && !/^[0-9a-f-]{36}$/i.test(node) && !/^\d{4}-\d{2}-\d{2}T/.test(node)) {
out.push({ path, text: node });
}
return out;
@ -40,8 +38,8 @@ export function extractFinalResponseMessage(eventData: unknown): string | null {
// response_text XML fallback
const raw = walkStrings(eventData).find((t) => /response_text$/i.test(t.path));
if (raw) {
const m = raw.text.match(/<final_response>\s*([\s\S]*?)\s*<\/final_response>/i);
if (m) return m[1]!.trim();
const [block] = findTagBlocks(raw.text, /<final_response>/gi, /<\/final_response>/gi);
if (block) return block.inner.trim();
}
return null;
}

View file

@ -24,10 +24,12 @@ import {
} from "./chatCore/contextEstimation.ts";
import {
extractSystemRoleMessages,
hoistLeadingTextSystemMessages,
relocateDirectiveOnlyMessages,
} from "./chatCore/claudeSystemRole.ts";
export {
extractSystemRoleMessages,
hoistLeadingTextSystemMessages,
relocateDirectiveOnlyMessages,
} from "./chatCore/claudeSystemRole.ts";
import { checkIdempotencyCache } from "./chatCore/idempotency.ts";
@ -2507,6 +2509,10 @@ async function handleChatCoreInner({
// messages[], but a directive-only message (content: [] +
// output_config) at messages[0] is rejected by Anthropic. Move it past
// the first real turn; Anthropic accepts the form at any other position.
// A text-bearing system message at messages[0] (e.g. the Output Styles
// injection) is rejected there too: hoist the leading run into the
// top-level `system` parameter first.
hoistLeadingTextSystemMessages(translatedBody);
relocateDirectiveOnlyMessages(translatedBody);
}
if (Array.isArray(translatedBody.messages)) {

View file

@ -167,3 +167,58 @@ export function extractSystemRoleMessages(payload: Record<string, unknown>): voi
}
payload.messages = messages.filter((m) => !isSystemRole(m.role));
}
/**
* Hoists the leading run of text-bearing system-role messages (everything
* before the first real user/assistant turn) into the top-level `system`
* parameter. Anthropic treats `messages[0]` as the initial system prompt
* position and rejects any non-directive system-role message there ("use the
* top-level 'system' parameter for the initial system prompt"), which is
* exactly where the Output Styles injection lands on the mid-conversation
* system passthrough (provider `claude` + 1M-context models). Only the leading
* run is hoisted so genuine mid-conversation system turns keep their position
* and cache prefix; empty (directive-only) messages in the run are left in
* place for relocateDirectiveOnlyMessages to handle.
*/
export function hoistLeadingTextSystemMessages(payload: Record<string, unknown>): void {
if (!Array.isArray(payload.messages) || payload.messages.length === 0) return;
const messages = payload.messages as Array<Record<string, unknown>>;
const isSystemRole = (role: unknown): boolean =>
typeof role === "string" &&
(role.toLowerCase() === "system" || role.toLowerCase() === "developer");
const blocks: Array<Record<string, unknown>> = [];
const kept: Array<Record<string, unknown>> = [];
let i = 0;
for (; i < messages.length; i++) {
const m = messages[i];
if (m == null || typeof m !== "object" || !isSystemRole(m.role)) break;
if (typeof m.content === "string") {
if (m.content.length > 0) blocks.push({ type: "text", text: m.content });
continue;
}
if (Array.isArray(m.content) && m.content.length > 0) {
let hoisted = false;
for (const block of m.content as Array<Record<string, unknown>>) {
if (block?.type === "text" && typeof block.text === "string" && block.text.length > 0) {
blocks.push({ type: "text", text: block.text });
hoisted = true;
}
}
if (!hoisted) kept.push(m);
continue;
}
kept.push(m);
}
if (blocks.length === 0) return;
const existing = payload.system;
if (typeof existing === "string" && existing.length > 0) {
payload.system = [{ type: "text", text: existing }, ...blocks];
} else if (Array.isArray(existing)) {
payload.system = [...(existing as Array<Record<string, unknown>>), ...blocks];
} else {
payload.system = blocks;
}
payload.messages = [...kept, ...messages.slice(i)];
}

View file

@ -55,7 +55,7 @@ import { promisify } from "node:util";
import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { parse as parseYaml } from "yaml";
import { load as parseYaml } from "js-yaml";
const __dirname = dirname(fileURLToPath(import.meta.url));
const ROOT = join(__dirname, "..", "..");

View file

@ -0,0 +1,33 @@
import { afterEach, describe, expect, it } from "vitest";
import { _injectPipeline, embedTransformers } from "../embedding/transformersLocal";
describe("optional Transformers backend in UI tests", () => {
afterEach(() => {
_injectPipeline(null);
});
it("reports an unavailable backend without downloading a model", async () => {
_injectPipeline(null);
const result = await embedTransformers("offline test");
expect(result).toMatchObject({
source: "transformers",
reason: "model_load_failed",
message: "Optional Transformers runtime is unavailable in UI unit tests",
});
});
it("uses an injected pipeline without the optional runtime", async () => {
_injectPipeline(async () => ({ data: new Float32Array([1, 2]), dims: [1, 2] }));
const result = await embedTransformers("offline test");
expect(result).toMatchObject({ source: "transformers", dimensions: 2 });
if (!("vector" in result)) {
throw new Error(`Expected an embedding vector, received ${result.reason}`);
}
expect(Array.from(result.vector)).toEqual([1, 2]);
});
});

View file

@ -1,16 +1,16 @@
/**
* Transformers.js local embedding (D8) — Xenova/all-MiniLM-L6-v2.
* Transformers.js local embedding (D8) - Xenova/all-MiniLM-L6-v2.
*
* IMPORTANT: @huggingface/transformers is imported lazily (await import())
* ONLY when this function is called. Never imported at module level.
* This satisfies D8 + D25 (serverExternalPackages + no bundle impact).
* IMPORTANT: @huggingface/transformers is imported lazily, ONLY when this
* function is called. Never imported at module level. The specifier is assembled
* at runtime and marked webpackIgnore so Next does not resolve the optional
* package while compiling /health (D8 + D25).
*/
import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts";
import type { EmbeddingResult, EmbeddingError } from "./types";
const TRANSFORMERS_MODEL =
process.env.MEMORY_TRANSFORMERS_MODEL || "Xenova/all-MiniLM-L6-v2";
const TRANSFORMERS_MODEL = process.env.MEMORY_TRANSFORMERS_MODEL || "Xenova/all-MiniLM-L6-v2";
// Singleton pipeline, initialized once
type PipelineFn = (text: string | string[], options?: Record<string, unknown>) => Promise<unknown>;
@ -28,9 +28,17 @@ async function getOrLoadPipeline(): Promise<PipelineFn> {
if (_pipelineLoading) return _pipelineLoading;
_pipelineLoading = (async (): Promise<PipelineFn> => {
// Lazy import — never at module level (D8, D25)
const transformers = await import("@huggingface/transformers");
const { pipeline } = transformers as { pipeline: (task: string, model: string, opts?: Record<string, unknown>) => Promise<PipelineFn> };
// Lazy import - never at module level (D8, D25). A string-literal
// specifier is still traced by Next even inside await import().
const specifier = "@huggingface/" + "transformers";
const transformers = await import(/* webpackIgnore: true */ specifier);
const { pipeline } = transformers as {
pipeline: (
task: string,
model: string,
opts?: Record<string, unknown>
) => Promise<PipelineFn>;
};
const pipe = await pipeline("feature-extraction", TRANSFORMERS_MODEL, { dtype: "q8" });
_pipeline = pipe;
_pipelineLoading = null;
@ -70,7 +78,7 @@ function tensorToFloat32Array(output: unknown): Float32Array {
seqLen = dims[0];
hiddenSize = dims[1];
} else {
// Already flat — return as-is
// Already flat - return as-is
return data instanceof Float32Array ? data : new Float32Array(data);
}

View file

@ -0,0 +1,4 @@
// UI unit tests must not download models or initialize the native ONNX runtime.
export async function pipeline(): Promise<never> {
throw new Error("Optional Transformers runtime is unavailable in UI unit tests");
}

View file

@ -92,7 +92,7 @@ describe("Protocol clients E2E", () => {
method: "PATCH",
body: JSON.stringify({ a2aEnabled: true }),
});
expect([200, 401]).toContain(response.status);
expect([200, 401, 403]).toContain(response.status);
});
it(

View file

@ -105,10 +105,20 @@ test("every @huggingface/transformers consumer loads it lazily so absent install
/^\s*import\s+(?:[^'"]*?\s+from\s+)?["']@huggingface\/transformers["']/m,
"transformersLocal.ts must not statically import @huggingface/transformers"
);
assert.doesNotMatch(
embeddingSrc,
/import\(\s*["']@huggingface\/transformers["']\s*\)/,
"transformersLocal.ts must not pass a literal specifier to import() - Next traces it at compile time"
);
assert.match(
embeddingSrc,
/await import\(["']@huggingface\/transformers["']\)/,
"transformersLocal.ts must load @huggingface/transformers via await import()"
/webpackIgnore:\s*true/,
"transformersLocal.ts must mark the optional import webpackIgnore so Next does not resolve it during /health"
);
assert.match(
embeddingSrc,
/["']@huggingface\/["']\s*\+\s*["']transformers["']/,
"transformersLocal.ts must assemble the specifier at runtime"
);
const workerSrc = readFileSync(

View file

@ -0,0 +1,77 @@
import assert from "node:assert/strict";
import { test } from "node:test";
import {
hoistLeadingTextSystemMessages,
relocateDirectiveOnlyMessages,
} from "../../open-sse/handlers/chatCore/claudeSystemRole.ts";
// Reproduction of the production 400 (2026-09-03/04): Output Styles injects a
// text system message at messages[0]; on the mid-conversation-system passthrough
// Anthropic rejects it ("use the top-level 'system' parameter for the initial
// system prompt").
test("hoistLeadingTextSystemMessages moves a text system messages[0] into top-level system", () => {
const payload: Record<string, unknown> = {
system: [{ type: "text", text: "You are Claude." }],
output_config: { effort: "medium" },
messages: [
{ role: "system", content: "[OmniRoute Output Styles]\nRespond terse." },
{ role: "user", content: "Reply exactly: MIDCONV_TOPLEVEL_OK" },
],
};
hoistLeadingTextSystemMessages(payload);
assert.deepEqual(payload.system, [
{ type: "text", text: "You are Claude." },
{ type: "text", text: "[OmniRoute Output Styles]\nRespond terse." },
]);
assert.equal((payload.messages as Array<{ role: string }>)[0].role, "user");
assert.equal((payload.messages as unknown[]).length, 1);
});
test("hoistLeadingTextSystemMessages converts a string top-level system and keeps mid-conversation system turns", () => {
const payload: Record<string, unknown> = {
system: "base",
messages: [
{ role: "system", content: [{ type: "text", text: "style" }] },
{ role: "user", content: "hello" },
{ role: "system", content: "mid-conversation context" },
{ role: "assistant", content: "hi" },
],
};
hoistLeadingTextSystemMessages(payload);
assert.deepEqual(payload.system, [
{ type: "text", text: "base" },
{ type: "text", text: "style" },
]);
const roles = (payload.messages as Array<{ role: string }>).map((m) => m.role);
assert.deepEqual(roles, ["user", "system", "assistant"]);
});
test("hoistLeadingTextSystemMessages leaves directive-only messages for relocateDirectiveOnlyMessages", () => {
const payload: Record<string, unknown> = {
messages: [
{ role: "system", content: [], output_config: { effort: "high" } },
{ role: "system", content: "style" },
{ role: "user", content: "hello" },
{ role: "assistant", content: "hi" },
],
};
hoistLeadingTextSystemMessages(payload);
assert.deepEqual(payload.system, [{ type: "text", text: "style" }]);
relocateDirectiveOnlyMessages(payload);
const msgs = payload.messages as Array<Record<string, unknown>>;
assert.equal(msgs[0].role, "user");
assert.equal(msgs[1].role, "system");
assert.deepEqual(msgs[1].output_config, { effort: "high" });
assert.equal(msgs[2].role, "assistant");
});
test("hoistLeadingTextSystemMessages is a no-op for a normal user first message", () => {
const payload: Record<string, unknown> = {
system: "base",
messages: [{ role: "user", content: "hello" }],
};
hoistLeadingTextSystemMessages(payload);
assert.equal(payload.system, "base");
assert.equal((payload.messages as unknown[]).length, 1);
});

View file

@ -0,0 +1,122 @@
import test from "node:test";
import assert from "node:assert/strict";
import {
extractBareSummaryEnvelope,
parseDevinToolRequest,
} from "../../open-sse/executors/devin-agentic/toolParser.ts";
import { extractFinalResponseMessage } from "../../open-sse/executors/promptql/eventTree.ts";
const OLD_SUMMARY = /^<summary>\s*([\s\S]*?)\s*<\/summary>$/i;
const OLD_FINAL = /<final_response>\s*([\s\S]*?)\s*<\/final_response>/i;
// Small enough to finish with the old cubic regexes, large enough that they would take
// seconds if these paths still used them (2,000 spaces cost the old patterns about 3 s each).
const RUN = 20_000;
const BUDGET_MS = 500;
function timed<T>(fn: () => T): { value: T; ms: number } {
const started = performance.now();
const value = fn();
return { value, ms: performance.now() - started };
}
test("bare summary envelope: same result as the old regex on ordinary input", () => {
const samples = [
"<summary>hello</summary>",
" <SUMMARY>\n spaced out \n</SUMMARY> ",
"<summary></summary>",
"<summary> </summary>",
"<summary>a</summary> tail",
"head <summary>a</summary>",
"<summary>a</summary><summary>b</summary>",
"<summary>only open",
"</summary>",
"<summary></summary",
"no envelope",
];
for (const sample of samples) {
const old = sample.trim().match(OLD_SUMMARY);
assert.equal(extractBareSummaryEnvelope(sample), old ? old[1].trim() : null, sample);
}
});
test("bare summary envelope: a long run of spaces is scanned in linear time", () => {
const open = timed(() => extractBareSummaryEnvelope(`<summary>${" ".repeat(RUN)}x`));
assert.equal(open.value, null);
assert.ok(open.ms < BUDGET_MS, `unclosed took ${open.ms}ms`);
const closed = timed(() => extractBareSummaryEnvelope(`<summary>${" ".repeat(RUN)}x</summary>`));
assert.equal(closed.value, "x");
assert.ok(closed.ms < BUDGET_MS, `closed took ${closed.ms}ms`);
});
test("devin tool request: well-formed envelope still parses", () => {
const tools = [
{
name: "read_file",
description: "",
input_schema: {
type: "object",
properties: { path: { type: "string" } },
required: ["path"],
},
},
];
const call = parseDevinToolRequest(
`\n<tool>\n {"name":"read_file","arguments":{"path":"a.txt"}}\n</tool>\n`,
tools as never
) as { name?: string } | null;
assert.ok(call);
assert.equal(parseDevinToolRequest("no tool here", tools as never), null);
assert.throws(
() =>
parseDevinToolRequest(
`hi <tool>{"name":"read_file","arguments":{"path":"a"}}</tool>`,
tools as never
),
/standalone tool envelope/
);
assert.throws(
() => parseDevinToolRequest("<tool>{}</tool><tool>{}</tool>", tools as never),
/more than one tool request/
);
});
test("devin tool request: unclosed tags and long whitespace are scanned in linear time", () => {
for (const text of [
`<tool>${" ".repeat(RUN)}x`,
"<tool>".repeat(RUN),
`<tool>${"\n ".repeat(RUN)}`,
]) {
const { value, ms } = timed(() => parseDevinToolRequest(text, []));
assert.equal(value, null);
assert.ok(ms < BUDGET_MS, `took ${ms}ms`);
}
});
test("promptql final response: same result as the old regex on ordinary input", () => {
const samples = [
"<final_response>hello</final_response>",
"x <FINAL_RESPONSE>\n spaced \n</FINAL_RESPONSE> y",
"<final_response></final_response>",
"<final_response>a</final_response><final_response>b</final_response>",
"<final_response>unclosed",
"no tag",
];
for (const sample of samples) {
const old = sample.match(OLD_FINAL);
const got = extractFinalResponseMessage({ response_text: sample });
assert.equal(got, old ? old[1].trim() : null, sample);
}
});
test("promptql final response: unclosed tag and long whitespace are scanned in linear time", () => {
for (const text of [
`<final_response>${" ".repeat(RUN)}x`,
"<final_response>".repeat(RUN),
`<final_response>${" ".repeat(RUN)}x</final_response>`,
]) {
const { ms } = timed(() => extractFinalResponseMessage({ response_text: text }));
assert.ok(ms < BUDGET_MS, `took ${ms}ms`);
}
});

View file

@ -409,6 +409,16 @@ test("extractCiGates: the REAL ci.yml yields the base-reds that leaked in v3.8.4
assert.ok(ids.size >= 20, "the real gate set is substantial (>= 20 static gates)");
});
test("validate-release-green parses workflow YAML via the declared js-yaml dependency", async () => {
const fs = await import("node:fs");
const src = fs.readFileSync(
new URL("../../scripts/quality/validate-release-green.mjs", import.meta.url),
"utf8"
);
assert.match(src, /from ["']js-yaml["']/);
assert.doesNotMatch(src, /from ["']yaml["']/);
});
// ─── Verdict accuracy (review of the #9985 release-green verdict) ────────────
test("firstFailureLine never blames a PASSING line whose test FILE NAME contains 'fail' (#9985)", () => {

View file

@ -46,6 +46,11 @@ export default defineConfig({
plugins: [react()],
resolve: {
alias: {
// Optional native embeddings are outside the UI unit-test runtime.
"@huggingface/transformers": path.resolve(
__dirname,
"./tests/_setup/transformersUnavailable.ts"
),
"@": path.resolve(__dirname, "./src"),
// Mirrors tsconfig paths. Without it, a UI test importing from open-sse
// resolves to undefined instead of failing loudly — which silently made