mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(compression): bound mcpAccessibility maxTextChars on the live read path (#4206)
* fix(compression): bound mcpAccessibility maxTextChars on the live read path
smartFilterText reserves 300 chars for the truncation tail, so a maxTextChars at/below that
makes headSize <= 0 and the whole tool result is replaced by the notice (total data loss). Two
gaps let a bad value reach the engine: the DB normalizer floored maxTextChars only at > 0, and
the live MCP-server read path (readMcpAccessibilityConfig) did a raw {...DEFAULT, ...parsed}
spread with no bounding at all.
Centralize the floors in a shared clampMcpAccessibilityConfig (engine layer) used by both the DB
normalizer and the server read path; values in (0, 600) fall back to the default. Export the
tail-reserve constant (300) and use it in smartFilterText so the engine and the bounds stay in
sync. The F5.3 headSize >= 0 clamp stays as a second layer.
Defense-in-depth today (the config isn't writable via the settings API yet — making it
configurable is a tracked follow-up in the compression-completeness work); this closes the
data-loss path for when it lands.
Part of the compression "100% functional" program (audit follow-up).
* chore(quality): bump server.ts file-size baseline 1457->1458 (#4206 +1 import)
This commit is contained in:
committed by
GitHub
parent
aed2a5024c
commit
4fecb34ac6
@@ -13,6 +13,7 @@
|
||||
"_rebaseline_2026_06_18_qg9_combo_split_d8": "QG v2 Fase 9 T5 D8 (reduced): combo.ts 3819->3432 — auto-strategy scoring/intent/tag-routing extracted byte-identically to the new open-sse/services/combo/autoStrategy.ts (434, <cap): the quota-soft execution-candidate registry (the single _activeExecutionCandidates Map + setCandidateQuotaSoftPenalty/_registerExecutionCandidates/_unregisterExecutionCandidates, kept together for state cohesion), QUOTA_SOFT_DEPRIORITIZE_FACTOR, scoreAutoTargets, expandAutoComboCandidatePool, intent extraction (toTextContent/extractPromptForIntent/mapIntentToTaskType/toStringArray/getIntentConfig), applyRequestTagRouting and deriveComboSessionKey. buildAutoCandidates + its two private-only helpers calculateTargetContextAffinity/getBootstrapLatencyMs (and the DEFAULT_MODEL_P95_MS/MIN_HISTORY_SAMPLES/OUTPUT_TOKEN_RATIO consts they own) were DELIBERATELY KEPT in combo.ts: buildAutoCandidates is the sole user of the internal reset-window helpers (resolveResetWindowConfig/fetchResetAwareQuotaWithCache/calculateResetWindowAffinity + the ResetWindowConfig type), so keeping it there leaves those helpers private (no export) and avoids a combo <-> autoStrategy import cycle (a later task can move the reset-window block out of combo.ts without breaking autoStrategy). autoStrategy.ts never imports from the combo barrel. dedupeTargetsByExecutionKey was already in comboData.ts (D6) and is NOT re-moved. QUOTA_SOFT_DEPRIORITIZE_FACTOR + setCandidateQuotaSoftPenalty stay re-exported from combo.ts for chatCore.ts's dynamic import(\"../services/combo\"); scoreAutoTargets/expandAutoComboCandidatePool keep their public re-export too. Orphaned combo.ts imports dropped where moved-out symbols stopped being referenced; ProviderCandidate/AutoProviderCandidate/HistoricalLatencyStatsEntry types are imported into combo.ts for buildAutoCandidates. Pure move, no logic change.",
|
||||
"_rebaseline_2026_06_18_qg9_combo_split_d7a": "QG v2 Fase 9 T5 D7a: combo.ts 3440->3398 (wc -l 3397 + 1, stacked on #4194 which added the passthrough-quota guards) — round-robin sticky state extracted byte-identically to the new open-sse/services/combo/rrState.ts (71, <cap). Moved exactly the 6 RR-state symbols: MAX_RR_COUNTERS, the two mutable module Maps rrCounters (new Map<string,number>) + rrStickyTargets, and the three helpers clampStickyRoundRobinTargetLimit/getStickyRoundRobinStartIndex/recordStickyRoundRobinSuccess. STATE COHESION: rrCounters and rrStickyTargets remain SINGLE instances defined once in rrState.ts; combo.ts imports the same references back and keeps mutating them directly in orderTargetsByResetAwareQuota/orderTargetsByResetWindow/handleRoundRobinCombo (no Map duplicated — sticky-3 round-robin behavior preserved, guarded by combo-routing-e2e.test.ts). The quota state left behind (MAX_RESET_AWARE_CACHE/resetAwareConnectionCache/resetAwareQuotaCache) stays in combo.ts for a later task (D7b). None of the 6 were ever public, so combo.ts imports all six back (no re-export). rrState.ts never imports from the combo barrel; it imports only ResolvedComboTarget from ./types.ts. Pure move, no logic change.",
|
||||
"_rebaseline_2026_06_18_qg9_combo_split_d7b": "QG v2 Fase 9 T5 D7b: combo.ts 3398->2597 (wc -l 2596 + 1) — the reset-aware / reset-window quota block extracted byte-identically into two new leaves under open-sse/services/combo/ (both <cap). Split was size-driven (~818 source LOC > 800 cap): quotaScoring.ts (311) holds the PURE half (config consts/resolvers + window-math/scoring helpers, no state, no async); quotaStrategies.ts (568) holds the STATEFUL/async half — the two mutable caches resetAwareConnectionCache + resetAwareQuotaCache (new Map) + MAX_RESET_AWARE_CACHE kept as SINGLE instances next to their only readers/writers getQuotaAwareConnectionsForTarget + fetchResetAwareQuotaWithCache (STATE COHESION; grep 'resetAware*Cache = new Map' in combo.ts == 0), plus normalizeConnectionIds/filterAllowedConnectionIds/getTargetConnectionIds/mapWithConcurrency/preScreenTargets/orderTargetsByResetAwareQuota/orderTargetsByResetWindow. quotaStrategies imports rrCounters + MAX_RR_COUNTERS from ./rrState.ts (D7a) and the pure helpers from ./quotaScoring.ts; neither leaf imports the combo barrel. combo.ts imports back the 3 reset-window helpers buildAutoCandidates needs (resolveResetWindowConfig/fetchResetAwareQuotaWithCache/calculateResetWindowAffinity) + resolveSlaRoutingPolicy + the 3 orderers used by the orchestrator; preScreenTargets stays re-exported. Orphaned imports dropped (clamp01/PRE_SCREEN_CONCURRENCY/ProviderProfile/SlaRoutingPolicy/IsModelAvailable); hasPerModelQuota KEPT (used by the #4194 passthrough-quota guards that stay). buildAutoCandidates + calculateTargetContextAffinity/getBootstrapLatencyMs stay (D8). Pure move, no logic change.",
|
||||
"_rebaseline_2026_06_18_4206_mcpaccess_bounds": "PR #4206 own growth: open-sse/mcp-server/server.ts 1457->1458 (+1 = one named import `clampMcpAccessibilityConfig` added to the existing constants.ts import block in readMcpAccessibilityConfig). The fix bounds the LIVE read path (a persisted out-of-range maxTextChars made smartFilterText truncate the whole tool result away); all real logic lives in the new clampMcpAccessibilityConfig helper in engines/mcpAccessibility/constants.ts (<cap). Thin one-import wiring at the single correct read point; not extractable.",
|
||||
"_rebaseline_2026_06_18_8_2_sliding_window": "Fase 8.2 own growth: rateLimitManager.ts 1017->1022 (+5 = one import + one `await awaitProviderDefaultSlot(...)` call + a 2-line comment at the existing withRateLimit chokepoint). All sliding-window logic was extracted to the new open-sse/services/providerDefaultRateLimit.ts + open-sse/services/slidingWindowLimiter.ts (both <cap), NOT inlined. Thin wiring only; not further shrinkable.",
|
||||
"_rebaseline_2026_06_18_8_1_no_thinking_alias": "Fase 8.1 own growth: catalog.ts 1435->1440 (+5 = appendNoThinkingVariants(finalModels) call + comment at the existing finalModels chokepoint) and chat.ts 1458->1471 (+13 = applyNoThinkingAlias(body) call + comment right after body.model is read, before model resolution). All real logic lives in the new open-sse/utils/noThinkingAlias.ts (<cap); both edits are thin wiring of tested helpers at the single correct integration point in each file. Not extractable.",
|
||||
"_rebaseline_2026_06_18_4_4_midstream": "Fase 4.4 own growth: chatCore.ts 5980->6009 (+29 at the existing streaming-return chokepoint = capture streamRecovery.continueMidStream alongside .enabled; refactor the early-retry reopen thunk into a shared runUpstreamStream(body) helper — net DRY — and add the gated continueStream(assistantSoFar) thunk that re-runs the upstream with makeContinuationBody(bodyToSend, …), plus the onContinue log). All continuation logic (scanOpenAiSseText, makeContinuationBody, trimContinuationOverlap, the createRecoverableStream continuation path) lives in open-sse/services/streamRecovery.ts (<cap). Closes over the same per-attempt executor locals as the existing reopen; not extractable without hiding the dispatch boundary. Opt-in (default OFF).",
|
||||
@@ -72,7 +73,7 @@
|
||||
"open-sse/handlers/sseParser.ts": 812,
|
||||
"open-sse/handlers/videoGeneration.ts": 1078,
|
||||
"open-sse/mcp-server/schemas/tools.ts": 1437,
|
||||
"open-sse/mcp-server/server.ts": 1457,
|
||||
"open-sse/mcp-server/server.ts": 1458,
|
||||
"open-sse/mcp-server/tools/advancedTools.ts": 1118,
|
||||
"open-sse/services/accountFallback.ts": 1727,
|
||||
"open-sse/services/batchProcessor.ts": 828,
|
||||
|
||||
@@ -88,6 +88,7 @@ import { compressMcpRegistryMetadata } from "./descriptionCompressor.ts";
|
||||
import { smartFilterText } from "../services/compression/engines/mcpAccessibility/index.ts";
|
||||
import {
|
||||
DEFAULT_MCP_ACCESSIBILITY_CONFIG,
|
||||
clampMcpAccessibilityConfig,
|
||||
type McpAccessibilityConfig,
|
||||
} from "../services/compression/engines/mcpAccessibility/constants.ts";
|
||||
import { getDbInstance } from "../../src/lib/db/core.ts";
|
||||
@@ -137,9 +138,9 @@ function readMcpAccessibilityConfig(): McpAccessibilityConfig {
|
||||
.prepare("SELECT value FROM key_value WHERE namespace = ? AND key = ?")
|
||||
.get("compression", "mcpAccessibility") as { value?: string } | undefined;
|
||||
if (!row?.value) return { ...DEFAULT_MCP_ACCESSIBILITY_CONFIG };
|
||||
const parsed = JSON.parse(row.value);
|
||||
if (!parsed || typeof parsed !== "object") return { ...DEFAULT_MCP_ACCESSIBILITY_CONFIG };
|
||||
return { ...DEFAULT_MCP_ACCESSIBILITY_CONFIG, ...parsed };
|
||||
// clampMcpAccessibilityConfig bounds every field (and folds in the non-object guard), so a
|
||||
// persisted out-of-range maxTextChars can't make smartFilterText truncate the whole text.
|
||||
return clampMcpAccessibilityConfig(JSON.parse(row.value));
|
||||
} catch {
|
||||
return { ...DEFAULT_MCP_ACCESSIBILITY_CONFIG };
|
||||
}
|
||||
|
||||
@@ -24,3 +24,40 @@ export const DEFAULT_MCP_ACCESSIBILITY_CONFIG: McpAccessibilityConfig = {
|
||||
collapseKeepTail: MCP_ACCESSIBILITY_DEFAULTS.collapseKeepTail,
|
||||
minLengthToProcess: MCP_ACCESSIBILITY_DEFAULTS.minLengthToProcess,
|
||||
};
|
||||
|
||||
/**
|
||||
* Chars `smartFilterText` reserves for the truncation tail/notice (`maxTextChars - this` is the
|
||||
* head kept). A `maxTextChars` at or below this leaves no head, so the whole tool result would be
|
||||
* replaced by the notice. The engine and the config bounds must agree on this number.
|
||||
*/
|
||||
export const MCP_ACCESSIBILITY_TAIL_RESERVE = 300;
|
||||
|
||||
/**
|
||||
* Minimum sane `maxTextChars`: below this the truncated head is too small to be useful (or empty).
|
||||
* Values in `(0, MIN)` are treated as misconfiguration and fall back to the default.
|
||||
*/
|
||||
export const MCP_ACCESSIBILITY_MIN_MAX_TEXT_CHARS = MCP_ACCESSIBILITY_TAIL_RESERVE * 2;
|
||||
|
||||
function boundedInt(value: unknown, min: number, fallback: number): number {
|
||||
return typeof value === "number" && Number.isFinite(value) && value >= min
|
||||
? Math.floor(value)
|
||||
: fallback;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bound a raw/persisted mcpAccessibility config into a safe, fully-populated config. Centralizes
|
||||
* the numeric floors so both the DB normalizer and the live MCP-server read path agree (a small
|
||||
* `maxTextChars` would otherwise make `smartFilterText` truncate the whole text away).
|
||||
*/
|
||||
export function clampMcpAccessibilityConfig(raw: unknown): McpAccessibilityConfig {
|
||||
const record = (raw && typeof raw === "object" ? raw : {}) as Record<string, unknown>;
|
||||
const d = DEFAULT_MCP_ACCESSIBILITY_CONFIG;
|
||||
return {
|
||||
enabled: record["enabled"] !== false,
|
||||
maxTextChars: boundedInt(record["maxTextChars"], MCP_ACCESSIBILITY_MIN_MAX_TEXT_CHARS, d.maxTextChars),
|
||||
collapseThreshold: boundedInt(record["collapseThreshold"], 1, d.collapseThreshold),
|
||||
collapseKeepHead: boundedInt(record["collapseKeepHead"], 0, d.collapseKeepHead),
|
||||
collapseKeepTail: boundedInt(record["collapseKeepTail"], 0, d.collapseKeepTail),
|
||||
minLengthToProcess: boundedInt(record["minLengthToProcess"], 1, d.minLengthToProcess),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { collapseRepeated } from "./collapseRepeated.ts";
|
||||
import type { McpAccessibilityConfig } from "./constants.ts";
|
||||
import { MCP_ACCESSIBILITY_TAIL_RESERVE, type McpAccessibilityConfig } from "./constants.ts";
|
||||
|
||||
const NOISE_PATTERNS: RegExp[] = [/^\s*-\s*generic:?\s*$/gm, /^\s*-\s*text:\s*""\s*$/gm];
|
||||
|
||||
@@ -19,10 +19,11 @@ export function smartFilterText(text: string, config: McpAccessibilityConfig): s
|
||||
);
|
||||
|
||||
if (out.length > config.maxTextChars) {
|
||||
// Clamp to >=0: a maxTextChars below the 300-char tail reservation would make headSize
|
||||
// negative, and slice(0, negative) counts from the END — silently keeping a wrong,
|
||||
// oversized fragment instead of the intended head.
|
||||
const headSize = Math.max(0, config.maxTextChars - 300);
|
||||
// Clamp to >=0: a maxTextChars below the tail reservation would make headSize negative, and
|
||||
// slice(0, negative) counts from the END — silently keeping a wrong, oversized fragment
|
||||
// instead of the intended head. (clampMcpAccessibilityConfig keeps maxTextChars sane, but
|
||||
// smartFilterText is also called with raw configs in tests, so the clamp stays here too.)
|
||||
const headSize = Math.max(0, config.maxTextChars - MCP_ACCESSIBILITY_TAIL_RESERVE);
|
||||
const head = out.slice(0, headSize);
|
||||
// Measure omitted against the FILTERED text (out), not the raw input (text), which may
|
||||
// have shrunk via noise removal / collapse above.
|
||||
@@ -35,4 +36,8 @@ export function smartFilterText(text: string, config: McpAccessibilityConfig): s
|
||||
}
|
||||
|
||||
export type { McpAccessibilityConfig } from "./constants.ts";
|
||||
export { DEFAULT_MCP_ACCESSIBILITY_CONFIG } from "./constants.ts";
|
||||
export {
|
||||
DEFAULT_MCP_ACCESSIBILITY_CONFIG,
|
||||
clampMcpAccessibilityConfig,
|
||||
MCP_ACCESSIBILITY_TAIL_RESERVE,
|
||||
} from "./constants.ts";
|
||||
|
||||
@@ -340,4 +340,7 @@ export const DEFAULT_ULTRA_CONFIG: UltraConfig = {
|
||||
};
|
||||
|
||||
export type { McpAccessibilityConfig } from "./engines/mcpAccessibility/constants.ts";
|
||||
export { DEFAULT_MCP_ACCESSIBILITY_CONFIG } from "./engines/mcpAccessibility/constants.ts";
|
||||
export {
|
||||
DEFAULT_MCP_ACCESSIBILITY_CONFIG,
|
||||
clampMcpAccessibilityConfig,
|
||||
} from "./engines/mcpAccessibility/constants.ts";
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
DEFAULT_MCP_ACCESSIBILITY_CONFIG,
|
||||
DEFAULT_RTK_CONFIG,
|
||||
DEFAULT_ULTRA_CONFIG,
|
||||
clampMcpAccessibilityConfig,
|
||||
type AggressiveConfig,
|
||||
type CavemanConfig,
|
||||
type CavemanOutputModeConfig,
|
||||
@@ -519,32 +520,10 @@ export function getDefaultRtkConfig(): RtkConfig {
|
||||
}
|
||||
|
||||
function normalizeMcpAccessibilityConfig(value: unknown): McpAccessibilityConfig {
|
||||
const record = toRecord(value);
|
||||
return {
|
||||
...DEFAULT_MCP_ACCESSIBILITY_CONFIG,
|
||||
...record,
|
||||
enabled: record.enabled !== false,
|
||||
maxTextChars:
|
||||
typeof record.maxTextChars === "number" && record.maxTextChars > 0
|
||||
? Math.floor(record.maxTextChars)
|
||||
: DEFAULT_MCP_ACCESSIBILITY_CONFIG.maxTextChars,
|
||||
collapseThreshold:
|
||||
typeof record.collapseThreshold === "number" && record.collapseThreshold > 0
|
||||
? Math.floor(record.collapseThreshold)
|
||||
: DEFAULT_MCP_ACCESSIBILITY_CONFIG.collapseThreshold,
|
||||
collapseKeepHead:
|
||||
typeof record.collapseKeepHead === "number" && record.collapseKeepHead >= 0
|
||||
? Math.floor(record.collapseKeepHead)
|
||||
: DEFAULT_MCP_ACCESSIBILITY_CONFIG.collapseKeepHead,
|
||||
collapseKeepTail:
|
||||
typeof record.collapseKeepTail === "number" && record.collapseKeepTail >= 0
|
||||
? Math.floor(record.collapseKeepTail)
|
||||
: DEFAULT_MCP_ACCESSIBILITY_CONFIG.collapseKeepTail,
|
||||
minLengthToProcess:
|
||||
typeof record.minLengthToProcess === "number" && record.minLengthToProcess > 0
|
||||
? Math.floor(record.minLengthToProcess)
|
||||
: DEFAULT_MCP_ACCESSIBILITY_CONFIG.minLengthToProcess,
|
||||
};
|
||||
// clampMcpAccessibilityConfig (engine layer) owns the numeric floors so the DB normalizer and
|
||||
// the live MCP-server read path agree — in particular it floors maxTextChars to a sane minimum
|
||||
// (a value below the tail reservation would make smartFilterText truncate the whole text away).
|
||||
return clampMcpAccessibilityConfig(value);
|
||||
}
|
||||
|
||||
export async function getMcpAccessibilityConfig(): Promise<McpAccessibilityConfig> {
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
smartFilterText,
|
||||
clampMcpAccessibilityConfig,
|
||||
} from "../../../open-sse/services/compression/engines/mcpAccessibility/index.ts";
|
||||
import { DEFAULT_MCP_ACCESSIBILITY_CONFIG } from "../../../open-sse/services/compression/engines/mcpAccessibility/constants.ts";
|
||||
|
||||
// smartFilterText reserves 300 chars for the truncation tail/notice, so any maxTextChars below
|
||||
// that leaves headSize <= 0 and the whole tool result is replaced by the notice (total data
|
||||
// loss). The DB normalizer floored maxTextChars only at > 0, and the production read path in
|
||||
// server.ts bypassed bounding entirely. clampMcpAccessibilityConfig is the shared guard.
|
||||
test("clamps maxTextChars below the tail reserve to the default", () => {
|
||||
for (const bad of [1, 50, 300, 599]) {
|
||||
assert.equal(
|
||||
clampMcpAccessibilityConfig({ maxTextChars: bad }).maxTextChars,
|
||||
DEFAULT_MCP_ACCESSIBILITY_CONFIG.maxTextChars,
|
||||
`maxTextChars=${bad} must fall back to default`
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("keeps a sane maxTextChars (>= 600)", () => {
|
||||
assert.equal(clampMcpAccessibilityConfig({ maxTextChars: 600 }).maxTextChars, 600);
|
||||
assert.equal(clampMcpAccessibilityConfig({ maxTextChars: 1000 }).maxTextChars, 1000);
|
||||
assert.equal(clampMcpAccessibilityConfig({ maxTextChars: 1234.9 }).maxTextChars, 1234);
|
||||
});
|
||||
|
||||
test("bounds the other numeric fields and honors enabled", () => {
|
||||
const c = clampMcpAccessibilityConfig({
|
||||
collapseThreshold: -5,
|
||||
minLengthToProcess: 0,
|
||||
collapseKeepHead: -1,
|
||||
enabled: false,
|
||||
});
|
||||
assert.equal(c.collapseThreshold, DEFAULT_MCP_ACCESSIBILITY_CONFIG.collapseThreshold);
|
||||
assert.equal(c.minLengthToProcess, DEFAULT_MCP_ACCESSIBILITY_CONFIG.minLengthToProcess);
|
||||
assert.equal(c.collapseKeepHead, DEFAULT_MCP_ACCESSIBILITY_CONFIG.collapseKeepHead);
|
||||
assert.equal(c.enabled, false);
|
||||
});
|
||||
|
||||
test("a clamped config never lets smartFilterText truncate the whole text away", () => {
|
||||
// The previously-dangerous stored value: maxTextChars=50 → clamps to default, so a
|
||||
// 1000-char tool result is NOT replaced wholesale by the truncation notice.
|
||||
const cfg = clampMcpAccessibilityConfig({ maxTextChars: 50, minLengthToProcess: 1 });
|
||||
const out = smartFilterText("A".repeat(1000), cfg);
|
||||
assert.ok(out.includes("A".repeat(500)), "content preserved (not nuked by a tiny maxTextChars)");
|
||||
});
|
||||
Reference in New Issue
Block a user