From eea3c419c157849f04dd558d1aff264bb871ed8f Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 23 Aug 2026 07:25:33 +0000 Subject: [PATCH 1/2] refactor(responses): isolate fetch helper imports --- src/server/responses/fetch-helpers.ts | 99 +------------------ structure/04_transports-and-sidecars.md | 16 +++ .../responses-fetch-helpers-boundary.test.ts | 46 +++++++++ 3 files changed, 64 insertions(+), 97 deletions(-) create mode 100644 tests/responses-fetch-helpers-boundary.test.ts diff --git a/src/server/responses/fetch-helpers.ts b/src/server/responses/fetch-helpers.ts index 1a296b4037..eb82200cd0 100644 --- a/src/server/responses/fetch-helpers.ts +++ b/src/server/responses/fetch-helpers.ts @@ -5,107 +5,12 @@ import { shouldUseCodexWsUpstream, type BunRuntimeGateInput, } from "./ws-upstream"; -import { bridgeToResponsesSSE, buildResponseJSON, formatErrorResponse, type ResponsesTerminalStatus } from "../../bridge"; -import { - getConfigPath, - multiAgentGuidanceEnabled, - resolveEnvValue, -} from "../../config"; -import { parseRequest } from "../../responses/parser"; -import { buildCompactV1Output, COMPACT_PROMPT, decodeCompactionSummary, extractCompactUserMessages } from "../../responses/compaction"; -import { FORWARD_HEADERS, sanitizeReasoningInputContent } from "../../adapters/openai-responses"; -import { expandPreviousResponseInput, previousResponseProviderState, rememberResponseState } from "../../responses/state"; -import { routeModel } from "../../router"; -import { - advanceComboAfterFailure, - comboDefaultEffort, - comboFailureDecision, - comboIdFromRawBody, - concreteComboRequestBody, - getCombo, - isComboTargetInCooldown, - NoAvailableComboTargetsError, - noteComboSuccess, - parseRetryAfterMs, - pickComboTarget, - targetKey, -} from "../../combos"; -import { isInjectionDebugEnabled } from "../../lib/debug-settings"; -import { injectionDebugLog } from "../../lib/injection-debug-log"; -import { modelInList, namespacedToolName } from "../../types"; -import type { AdapterEvent, OcxConfig, OcxParsedRequest, OcxProviderConfig, OcxProviderContinuationState, OcxUsage } from "../../types"; -import { - forceRefreshOAuthAccessSnapshot, - getOAuthCredentialApiBaseUrl, - getOAuthCredentialProjectId, - getValidAccessTokenSnapshot, - type OAuthAccessSnapshot, - UnsupportedOAuthProviderError, -} from "../../oauth"; -import { buildWebSearchTool, planWebSearch, runWithWebSearch, shouldResolveOpenAiWebSearchSidecar } from "../../web-search"; -import { describeImagesInPlace, planVisionSidecar, shouldResolveOpenAiVisionSidecar, stripImagesInPlace } from "../../vision"; -import { createAdapterEventQueue, preflightAdapterEvents } from "../../adapters/run-turn-queue"; -import { - applyCodexAuthContextToProvider, - CodexAccountCooldownError, - CodexAuthContextError, - CodexDirectAuthenticationError, - CodexPoolAuthenticationError, - CodexThreadAffinityExpiredError, - headersForCodexAuthContext, - isCodexAuthContextUsable, - resolveCodexAuthContext, - type CodexAuthContext, -} from "../../codex/auth-context"; -import { - formatCodexProviderForLog, - recordCodexUpstreamOutcome, - type CodexUpstreamOutcome, -} from "../../codex/routing"; -import { fetchWithResetRetry, fetchWithTransientRetry, applyUpstreamRecoveryInit } from "../../lib/upstream-retry"; -import { ForwardAdmissionCredentialError, validateForwardAdmissionCredential } from "../auth-cors"; -import { listOpenAiForwardSidecarCandidates, resolveFirstUsableOpenAiSidecar, type ResolvedOpenAiForwardSidecar } from "../../providers/openai-sidecar"; -import { isCanonicalOpenAiForwardProvider } from "../../providers/openai-tiers"; -import { slugsEquivalent } from "../../providers/slug-codec"; -import { applyOpenAiVirtualModel, resolveOpenAiCompactModel } from "../../providers/openai-virtual-models"; -import { isUsageDebugEnabled } from "../../usage/debug"; -import { readJsonRequestBody, DecompressedBodyTooLargeError, UnsupportedContentEncodingError } from "../request-decompress"; -import { resolveAdapter, resolveWireProtocolOverride } from "../adapter-resolve"; -import { hasKeyPoolFailover, rotateProviderTransportOn429 } from "../../providers/key-failover"; -import { shouldAttemptImageTierRetry } from "../image-retry"; -import { resolveProviderTransport } from "../../providers/xai-transport"; +import type { OcxProviderConfig } from "../../types"; import type { WsData } from "../ws-bridge"; -import { registerTurn, trackStreamLifetime, unregisterTurn } from "../lifecycle"; -import { redactSecretString } from "../../lib/redact"; -import { readBoundedResponseBody } from "../../lib/bounded-body"; -import { supportedLadderFor } from "../effort-policy"; -import { - beginRequestAttempt, - catalogModelSupportsServiceTier, - finishRequestAttempt, - inspectResponseLogJson, - noteAttemptSend, - readConfiguredCodexServiceTier, - requestLogSpeedLabel, - sealRequestAttemptIdentity, - usageFromResponsesPayload, - type RequestLogContext, -} from "../request-log"; -import type { AttemptRecoveryKind } from "../../usage/log"; -import { - consumeForInspection, - consumeForResponseLogMetadata, - markNativePassthroughSseResponse, - relaySseWithFailedTail, - relayWithAbort, - sanitizePassthroughHeaders, -} from "../relay"; -import { hasResponsesItemIdRepair, relaySseWithResponsesItemIdRepair } from "../responses-item-id-repair"; -import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog"; import { waitForProviderRequestSlot } from "../../providers/request-pacing"; import { withUpstreamHttpVersion } from "../../lib/upstream-http-version"; -export { withUpstreamHttpVersion } from "../../lib/upstream-http-version"; +export { withUpstreamHttpVersion }; export function disableResponsesRequestTimeout(req: Request, server: Pick, "timeout"> | undefined): boolean { if (!server) return false; diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index eca817d9a4..3968d18ae2 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -39,6 +39,22 @@ executor contract. Main-request migration must not treat that branch as fixed-tr provider, lets the selected adapter speak the upstream protocol, then bridges adapter events back to Responses-compatible streaming output. +### Fetch-helper import boundary + +`src/server/responses/fetch-helpers.ts` is a transport leaf shared by Responses, compact, and native +Chat. Its runtime imports are limited to the Codex WebSocket transport, provider request pacing, and +the upstream HTTP-version helper. Server, provider, and WebSocket data types remain type-only edges. +It must not import routing, combos, OAuth, adapters, sidecars, response parsing, logging, or relay +modules merely because those imports existed in the pre-split `responses.ts` monolith. + +[Decision Log] +- 목적과 의도: Keep transport helpers reusable without making every consumer evaluate the full routed Responses and sidecar graph at module load. +- 기존 구현 및 제약 조건: The original `responses.ts` split copied the monolith import header into `fetch-helpers.ts`; seven helper exports therefore retained 39 distinct runtime import specifiers and reached 326 modules even though the implementations used only three runtime dependencies. +- 검토한 주요 대안: Leave the imports because current modules have limited top-level side effects; move the helpers again; prune the copied imports and lock the direct runtime boundary. +- 선택한 방식: Preserve the file and all public exports, remove unused runtime edges, and enforce an explicit three-specifier allowlist with a source-level regression that also proves type-only imports are ignored. +- 다른 대안 대신 이 방식을 선택한 이유: Relying on unrelated modules to remain side-effect-free makes startup ownership accidental, while another move adds churn without changing the responsibility boundary. +- 장점, 단점 및 영향: Ordinary native Chat and compact consumers no longer load unrelated routing, combo, OAuth, web-search, vision, and relay modules through this leaf. The allowlist is intentionally strict, so a future helper that needs a new runtime dependency must make that ownership decision explicit in code, tests, and this document. + [Decision Log] - 목적과 의도: Prevent routed models from turning invented or neighboring-agent tool names into client-executable Responses calls. - 기존 구현 및 제약 조건: The request catalog already controlled custom-tool restoration and the non-OpenAI prompt nudge, but an undeclared upstream name still fell through as an ordinary `function_call`; Codex then reduced the mismatch to a bare `aborted` result. diff --git a/tests/responses-fetch-helpers-boundary.test.ts b/tests/responses-fetch-helpers-boundary.test.ts new file mode 100644 index 0000000000..aede124946 --- /dev/null +++ b/tests/responses-fetch-helpers-boundary.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const helperPath = resolve(repoRoot, "src/server/responses/fetch-helpers.ts"); + +const RUNTIME_IMPORT_RE = /^\s*import\s+(?!type\b)[^;]*?from\s+["']([^"']+)["']|^\s*import\s+["']([^"']+)["']|^\s*export\s+(?!type\b)[^;]*?from\s+["']([^"']+)["']|\bimport\s*\(\s*["']([^"']+)["']\s*\)/gm; + +function runtimeImportSpecifiers(source: string): string[] { + const specifiers: string[] = []; + RUNTIME_IMPORT_RE.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = RUNTIME_IMPORT_RE.exec(source)) !== null) { + const specifier = match[1] ?? match[2] ?? match[3] ?? match[4]; + if (specifier) specifiers.push(specifier); + } + return [...new Set(specifiers)].sort(); +} + +describe("Responses fetch-helper import boundary", () => { + test("loads only transport-owned runtime dependencies", () => { + expect(runtimeImportSpecifiers(readFileSync(helperPath, "utf8"))).toEqual([ + "../../lib/upstream-http-version", + "../../providers/request-pacing", + "./ws-upstream", + ]); + }); + + test("the guard recognizes runtime edges and ignores type-only imports", () => { + expect(runtimeImportSpecifiers([ + 'import type { T } from "./types";', + 'export type { U } from "./other-types";', + 'import { a } from "./static";', + 'import "./side-effect";', + 'export { b } from "./re-export";', + 'const c = import("./dynamic");', + ].join("\n"))).toEqual([ + "./dynamic", + "./re-export", + "./side-effect", + "./static", + ]); + }); +}); From be6ea98a019541256b0d59dd0c082ae86724dac3 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Sun, 23 Aug 2026 07:53:32 +0000 Subject: [PATCH 2/2] test(responses): reject dynamic import bypasses --- .../responses-fetch-helpers-boundary.test.ts | 56 +++++++++++++++---- 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/tests/responses-fetch-helpers-boundary.test.ts b/tests/responses-fetch-helpers-boundary.test.ts index aede124946..b4bc7de6bf 100644 --- a/tests/responses-fetch-helpers-boundary.test.ts +++ b/tests/responses-fetch-helpers-boundary.test.ts @@ -2,26 +2,47 @@ import { describe, expect, test } from "bun:test"; import { readFileSync } from "node:fs"; import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { createScanner, LanguageVariant, SyntaxKind } from "typescript/unstable/ast"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const helperPath = resolve(repoRoot, "src/server/responses/fetch-helpers.ts"); -const RUNTIME_IMPORT_RE = /^\s*import\s+(?!type\b)[^;]*?from\s+["']([^"']+)["']|^\s*import\s+["']([^"']+)["']|^\s*export\s+(?!type\b)[^;]*?from\s+["']([^"']+)["']|\bimport\s*\(\s*["']([^"']+)["']\s*\)/gm; +interface RuntimeImportScan { + specifiers: string[]; + nonLiteralDynamicImports: string[]; +} + +const importTranspiler = new Bun.Transpiler({ loader: "ts" }); -function runtimeImportSpecifiers(source: string): string[] { - const specifiers: string[] = []; - RUNTIME_IMPORT_RE.lastIndex = 0; - let match: RegExpExecArray | null; - while ((match = RUNTIME_IMPORT_RE.exec(source)) !== null) { - const specifier = match[1] ?? match[2] ?? match[3] ?? match[4]; - if (specifier) specifiers.push(specifier); +function nonLiteralDynamicImports(source: string): string[] { + const scanner = createScanner(true, LanguageVariant.Standard, source); + const imports: string[] = []; + for (;;) { + const token = scanner.scan(); + if (token === SyntaxKind.EndOfFile) return imports; + if (token !== SyntaxKind.ImportKeyword) continue; + if (scanner.scan() !== SyntaxKind.OpenParenToken) continue; + const argument = scanner.scan(); + if (argument !== SyntaxKind.StringLiteral) imports.push(scanner.getTokenText()); } - return [...new Set(specifiers)].sort(); +} + +function runtimeImports(source: string): RuntimeImportScan { + return { + specifiers: [...new Set(importTranspiler.scanImports(source).map(item => item.path))].sort(), + nonLiteralDynamicImports: nonLiteralDynamicImports(source), + }; +} + +function expectRuntimeImportBoundary(source: string): string[] { + const scan = runtimeImports(source); + expect(scan.nonLiteralDynamicImports).toEqual([]); + return scan.specifiers; } describe("Responses fetch-helper import boundary", () => { test("loads only transport-owned runtime dependencies", () => { - expect(runtimeImportSpecifiers(readFileSync(helperPath, "utf8"))).toEqual([ + expect(expectRuntimeImportBoundary(readFileSync(helperPath, "utf8"))).toEqual([ "../../lib/upstream-http-version", "../../providers/request-pacing", "./ws-upstream", @@ -29,18 +50,29 @@ describe("Responses fetch-helper import boundary", () => { }); test("the guard recognizes runtime edges and ignores type-only imports", () => { - expect(runtimeImportSpecifiers([ + const scan = runtimeImports([ 'import type { T } from "./types";', + 'import { type T2 } from "./more-types";', 'export type { U } from "./other-types";', + 'export { type U2 } from "./more-other-types";', 'import { a } from "./static";', 'import "./side-effect";', 'export { b } from "./re-export";', 'const c = import("./dynamic");', - ].join("\n"))).toEqual([ + 'const moduleName = "./hidden";', + 'const d = import(moduleName);', + 'const e = import(`./template`);', + ].join("\n")); + expect(scan.specifiers).toEqual([ "./dynamic", "./re-export", "./side-effect", "./static", + "./template", + ]); + expect(scan.nonLiteralDynamicImports).toEqual([ + "moduleName", + "`./template`", ]); }); });