fix(sse): extract Vertex error classifier and rebaseline frozen file sizes

This commit is contained in:
Will Gordon
2026-07-30 18:41:25 -04:00
parent 77eb184f9d
commit ea801bbca2
3 changed files with 71 additions and 64 deletions

View File

@@ -1,4 +1,5 @@
{
"_rebaseline_2026_07_30_9006_vertex_claude_catalog_dispatch": "PR #9006 (fix/vertex-claude-catalog-dispatch): three files, two causes. (1) src/sse/handlers/chat.ts 1845->1846 (+1): NOT this PR's own growth — this PR never touches chat.ts at all. Measured 1846 (split(\"\\n\").length) at this PR's own merge-base (before any of its 11 commits), so the drift was already inherited from already-merged PRs on release/v3.8.50 (fast-gates PR->release do not run check:file-size, same root cause as _rebaseline_2026_07_25_v3849_basered_filesize and _rebaseline_2026_07_02_5798_release_green) — no offending branch left to fix. (2) src/sse/services/auth.ts 2508->2512 (+4 net, after extraction — see below) and open-sse/handlers/chatCore.ts 5020->5023 (+3, comment-only): genuine own growth. auth.ts adds Vertex 403 PERMISSION_DENIED disambiguation (Google's google.rpc.ErrorInfo proto distinguishes a connection-wide cause — SERVICE_DISABLED, or IAM_PERMISSION_DENIED against a project-level resource — from a model-specific one scoped to a .../models/<id> resource), added mid-PR after a quality-gate reviewer flagged the plan's originally-accepted \"Vertex 403 always -> per-model lockout\" trade-off. The actual classification logic (~40 lines) was EXTRACTED into a new leaf module src/sse/services/vertexErrorClassifier.ts (mirrors the googApiKeyAuth.ts precedent, _rebaseline_2026_07_14_7034_goog_api_key), leaving only the irreducible call-site wiring in the frozen file: a 1-line import plus widening the existing #3027 per-model-403 guard condition. chatCore.ts's +3 is a pure comment expansion (no functional change) clarifying that the adjacent effort-suffix strip is no longer unconditional for every provider, requested by a separate quality-gate code-reviewer finding; not extractable (it's a comment). Auth.ts's disambiguation logic covered by 3 new test cases in tests/unit/vertex-passthrough-model-lockout.test.ts (SERVICE_DISABLED, IAM_PERMISSION_DENIED+model-resource, IAM_PERMISSION_DENIED+project-resource) plus a 4th regression test for a multi-detail-body correlation bug (reason and resource must be read from the SAME ErrorInfo detail, not independently regexed across the whole body) found by an adversarial quality-gate pass and fixed before merge.",
"_rebaseline_2026_07_24_8470_hyperagent_sticky_thread": "PR #8470 (artickc, fix/hyperagent-tool-loop-thread-sticky) own growth: open-sse/executors/hyperagent.ts 936->1025 (wc -l; check-file-size.mjs counts via split(\"\\n\").length so the gate sees 937->1026, +89, crosses the 1000 cap). Fixes a real bug where a reverse-conversion proxy (text-Intent/JSON to Claude Code native tool_calls) rewrites assistant messages between agentic tool-loop turns, breaking HyperAgent's conversation-prefix fingerprint and cold-starting the thread mid tool-loop. Adds Anthropic tool_use/tool_result flattening to extractMessageText() plus a new rootUserFingerprint()/root-key lookup tier in resolveHyperAgentThreadBinding()/storeHyperAgentThreadAfterTurn() so the thread stays sticky across the tool loop. Cohesive additions inside the existing single-file executor; not extractable without splitting the executor mid-request-flow. Covered by tests/unit/executor-hyperagent.test.ts (19/19, +5 new cases for tool_result/tool_use flattening + root-key stickiness). Pre-merge review flagged a cross-conversation root-key collision risk (tracked in the PR's own mandatory pre-merge checklist, not yet addressed) — unrelated to this file-size ratchet, tracked separately by /fix-prs.",
"_rebaseline_2026_07_25_8494_capability_filter_fail_closed": "PR #8494 (fix/capability-filters-fail-closed, #8488) own growth: open-sse/services/combo.ts 3640->3693 (+53) adds a fail-closed guard after filterTargetsByRequestCompatibility() — when every eligible target is excluded by request-capability filtering (vision/tools/etc) instead of quota/health, the combo now returns an explicit `capability_mismatch` 400 (describeCapabilityFilterExhaustion, imported from combo/comboStructure.ts) rather than silently falling through to a generic no-targets error, plus a `compatFilterFailOpen` escape hatch (combo config OR settings) mirrored at both the main/auto and round-robin call sites for symmetry. combo/comboStructure.ts (previously under cap, un-frozen) grows 794->918 (+124) — new home for describeCapabilityFilterExhaustion + providerSupportsEmulatedToolCalling (#5240 emulated tool-calling exemption so fail-closed does not regress prompt-emulation-only combos like all-chatgpt-web). Irreducible orchestration wiring at the existing filter chokepoint (same precedent as #7301's universal-cooldown-retry generalization). Companion test tests/unit/combo-routing-engine.test.ts 3409->3449 (+40, fail-closed/fail-open coverage across both call sites) also rebaselined. Covered by tests/unit/8488-capability-filter-fail-closed.test.ts (new) + 95/95 passing across both files. Structural shrink of combo.ts tracked in #3501.",
"_rebaseline_2026_07_25_8499_ts7_result_union_predicates": "PR #8499 (backryun, chore/ts7-types-executor-scattered) own growth: muse-spark-web.ts 1396->1405 (+9, irreducible). Under this workspace's `strictNullChecks: false`, the boolean-literal discriminant on `GraphqlResult` (`{ ok: true } | { ok: false; error: string }`) narrows the positive `.ok===true` branch but leaves `!result.ok` at the full union under TS7, making `.error` unreachable to the checker at the two call sites (warmup, mode-switch). Fixed by adding a single `isGraphqlFailure()` type-predicate helper (doc comment + 3-line body) reused at both call sites instead of duplicating the predicate inline — not extractable to a shared module without splitting a single-file executor's local narrowing helper out of its own file. Covered by the existing muse-spark-web executor test suite (no behavior change, pure narrowing fix).",
@@ -349,7 +350,7 @@
"open-sse/executors/deepseek-web.ts": 1148,
"open-sse/executors/grok-web.ts": 1044,
"open-sse/executors/muse-spark-web.ts": 1405,
"open-sse/handlers/chatCore.ts": 5020,
"open-sse/handlers/chatCore.ts": 5023,
"open-sse/handlers/imageGeneration.ts": 3101,
"open-sse/handlers/responseSanitizer.ts": 1115,
"open-sse/handlers/search.ts": 1536,
@@ -400,8 +401,8 @@
"src/shared/components/RequestLoggerV2.tsx": 1629,
"src/shared/components/analytics/charts.tsx": 1035,
"src/shared/services/cliRuntime.ts": 1122,
"src/sse/handlers/chat.ts": 1845,
"src/sse/services/auth.ts": 2508,
"src/sse/handlers/chat.ts": 1846,
"src/sse/services/auth.ts": 2512,
"tests/unit/account-fallback-service.test.ts": 1572,
"tests/unit/provider-validation-specialty.test.ts": 2980,
"open-sse/executors/hyperagent.ts": 1026

View File

@@ -77,6 +77,7 @@ import { isNoAuthProviderBlockedBySettings } from "./noAuthProviderSettings";
import { resolveAccountProxiesFromRegistry } from "./noAuthProxyResolution";
import { getNoAuthHydrationProviderIds } from "./noAuthProviderSiblings";
import { getResource404Bypass } from "./requestResourceHealth";
import { isVertexConnectionWidePermissionDenied } from "./vertexErrorClassifier";
import * as log from "../utils/logger";
import { fisherYatesShuffle, getNextFromDeckSync } from "@/shared/utils/shuffleDeck";
@@ -1898,67 +1899,6 @@ export async function getProviderCredentialsWithQuotaPreflight(
}
}
/**
* Google's google.rpc.ErrorInfo proto reliably distinguishes a connection-wide
* PERMISSION_DENIED (API not enabled, or a project-level IAM denial) from a
* model-specific one (IAM denial scoped to a .../models/<id> resource) — see
* https://cloud.google.com/apis/design/errors#error_info. Only returns true on
* POSITIVE evidence of a connection-wide cause; any other shape (including a
* missing/malformed resource field) falls through to the existing per-model
* lockout behavior, since that's the safer default and the actual bug this
* plan fixes (avoid defaulting BACK toward the connection-wide cooldown this
* plan exists to avoid).
*
* Parses the body as JSON and inspects each ErrorInfo-shaped detail object so
* `reason` and `resource` are correlated within the SAME detail entry — a
* multi-detail error body (unusual but possible) must not let one detail's
* resource leak into another detail's reason check. Falls back to a permissive
* regex scan (pre-JSON-parsing behavior) only when the body isn't parseable
* JSON or doesn't contain a `details` array, since Vertex error bodies aren't
* guaranteed to always be well-formed JSON.
*/
function isVertexConnectionWidePermissionDenied(errorText: string | null | undefined): boolean {
if (!errorText) return false;
try {
const parsed = JSON.parse(errorText);
const details: unknown[] =
parsed?.error?.details ?? parsed?.details ?? (Array.isArray(parsed) ? parsed : []);
if (Array.isArray(details) && details.length > 0) {
for (const detail of details) {
if (!detail || typeof detail !== "object") continue;
const reason = (detail as Record<string, unknown>).reason;
if (reason === "SERVICE_DISABLED") return true;
if (reason === "IAM_PERMISSION_DENIED") {
const metadata = (detail as Record<string, unknown>).metadata;
const resource =
metadata && typeof metadata === "object"
? (metadata as Record<string, unknown>).resource
: undefined;
if (typeof resource === "string" && !resource.includes("/models/")) return true;
}
}
// Well-formed details array present but no detail matched a connection-wide
// pattern (e.g. IAM_PERMISSION_DENIED with a /models/ resource, or no
// recognized reason at all) — per-model lockout is correct, don't fall
// through to the regex heuristic (it would just re-derive the same answer
// less precisely, or worse, could false-positive on stray substrings).
return false;
}
} catch {
// Not parseable JSON — fall through to the regex heuristic below.
}
// Fallback for non-JSON or unexpected-shape error bodies (regex-based,
// pre-JSON-parsing heuristic — kept for robustness against malformed bodies).
if (/"reason"\s*:\s*"SERVICE_DISABLED"/.test(errorText)) return true;
if (/"reason"\s*:\s*"IAM_PERMISSION_DENIED"/.test(errorText)) {
const resourceMatch = errorText.match(/"resource"\s*:\s*"([^"]*)"/);
if (resourceMatch && !resourceMatch[1].includes("/models/")) return true;
}
return false;
}
/** Persist exponential-backoff state for an unavailable provider connection. */
export async function markAccountUnavailable(
connectionId: string,

View File

@@ -0,0 +1,66 @@
/**
* Google's google.rpc.ErrorInfo proto reliably distinguishes a connection-wide
* PERMISSION_DENIED (API not enabled, or a project-level IAM denial) from a
* model-specific one (IAM denial scoped to a .../models/<id> resource) — see
* https://cloud.google.com/apis/design/errors#error_info. Only returns true on
* POSITIVE evidence of a connection-wide cause; any other shape (including a
* missing/malformed resource field) falls through to the existing per-model
* lockout behavior, since that's the safer default and the actual bug this
* plan fixes (avoid defaulting BACK toward the connection-wide cooldown this
* plan exists to avoid).
*
* Parses the body as JSON and inspects each ErrorInfo-shaped detail object so
* `reason` and `resource` are correlated within the SAME detail entry — a
* multi-detail error body (unusual but possible) must not let one detail's
* resource leak into another detail's reason check. Falls back to a permissive
* regex scan (pre-JSON-parsing behavior) only when the body isn't parseable
* JSON or doesn't contain a `details` array, since Vertex error bodies aren't
* guaranteed to always be well-formed JSON.
*
* Extracted to its own module so the single call site in
* `./auth.ts::markAccountUnavailable()` stays thin wiring, without growing the
* frozen `auth.ts` file (`config/quality/file-size-baseline.json`).
*/
export function isVertexConnectionWidePermissionDenied(
errorText: string | null | undefined
): boolean {
if (!errorText) return false;
try {
const parsed = JSON.parse(errorText);
const details: unknown[] =
parsed?.error?.details ?? parsed?.details ?? (Array.isArray(parsed) ? parsed : []);
if (Array.isArray(details) && details.length > 0) {
for (const detail of details) {
if (!detail || typeof detail !== "object") continue;
const reason = (detail as Record<string, unknown>).reason;
if (reason === "SERVICE_DISABLED") return true;
if (reason === "IAM_PERMISSION_DENIED") {
const metadata = (detail as Record<string, unknown>).metadata;
const resource =
metadata && typeof metadata === "object"
? (metadata as Record<string, unknown>).resource
: undefined;
if (typeof resource === "string" && !resource.includes("/models/")) return true;
}
}
// Well-formed details array present but no detail matched a connection-wide
// pattern (e.g. IAM_PERMISSION_DENIED with a /models/ resource, or no
// recognized reason at all) — per-model lockout is correct, don't fall
// through to the regex heuristic (it would just re-derive the same answer
// less precisely, or worse, could false-positive on stray substrings).
return false;
}
} catch {
// Not parseable JSON — fall through to the regex heuristic below.
}
// Fallback for non-JSON or unexpected-shape error bodies (regex-based,
// pre-JSON-parsing heuristic — kept for robustness against malformed bodies).
if (/"reason"\s*:\s*"SERVICE_DISABLED"/.test(errorText)) return true;
if (/"reason"\s*:\s*"IAM_PERMISSION_DENIED"/.test(errorText)) {
const resourceMatch = errorText.match(/"resource"\s*:\s*"([^"]*)"/);
if (resourceMatch && !resourceMatch[1].includes("/models/")) return true;
}
return false;
}