From 590ad3c677bbccf903d7a31267711a2f43f3455a Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:47:13 +0900 Subject: [PATCH 1/7] fix(config): preserve disabled sidecars when targets retire --- docs-site/src/content/docs/guides/sidecars.md | 5 + gui/src/pages/claude-code-sidecar.ts | 20 +-- gui/src/pages/claude-manual-env.ts | 2 +- gui/tests/claude-code-sidecar.test.ts | 14 ++ src/config/removed-opencode-free.ts | 11 ++ .../management/agent-settings-routes.ts | 10 +- src/types/config.ts | 4 +- structure/config.md | 2 +- structure/gui-and-management-api.md | 2 +- .../config-removed-opencode-free.test.ts | 112 +++++++++++++- ...agement-sidecar-anthropic-instance.test.ts | 125 ++++++++++++++- tests/vision/mistral-vision-sidecar.test.ts | 145 ++++++++++++------ 12 files changed, 384 insertions(+), 68 deletions(-) diff --git a/docs-site/src/content/docs/guides/sidecars.md b/docs-site/src/content/docs/guides/sidecars.md index 4df1c0cfcf7..e25d49dbfe7 100644 --- a/docs-site/src/content/docs/guides/sidecars.md +++ b/docs-site/src/content/docs/guides/sidecars.md @@ -18,6 +18,11 @@ closed. Explicit `xai` requires a usable stored Grok OAuth account and does not requires both ChatGPT login auth and an enabled `forward` provider. ::: +On file load, a retired OpenCode Free model target in global or `claudeCode` overrides of +`visionSidecar` or `webSearchSidecar` is removed and that sidecar is explicitly disabled, retaining +its other settings. Select a supported target before re-enabling it. Healthy targets and the defaults +for absent sidecar sections are unchanged. + ### Additional web-search backends (explicit-only) Three more web-search backends exist beyond the ChatGPT and Claude paths. Each is diff --git a/gui/src/pages/claude-code-sidecar.ts b/gui/src/pages/claude-code-sidecar.ts index e21c537fc58..156f4e686c8 100644 --- a/gui/src/pages/claude-code-sidecar.ts +++ b/gui/src/pages/claude-code-sidecar.ts @@ -1,7 +1,7 @@ /** * Claude Code sidecar override helpers — keep Auto as an in-memory draft so * users can enter a model before persist. Empty Auto persists only when it - * carries an explicit pool edit. + * carries an explicit activation state or pool edit. */ import type { AnthropicInstanceId, SidecarOverride, VisionOverrideBackend } from "./claude-manual-env"; @@ -9,8 +9,9 @@ import type { AnthropicInstanceId, SidecarOverride, VisionOverrideBackend } from export type SidecarSelectValue = "inherit" | "auto" | VisionOverrideBackend; export type PersistedSidecarOverride = { - backend: VisionOverrideBackend | null; - model: string; + enabled?: boolean; + backend?: VisionOverrideBackend | null; + model?: string; anthropicInstance?: AnthropicInstanceId | null; }; @@ -38,28 +39,29 @@ export function applySidecarPoolChange(override: SidecarOverride | undefined, va return next; } -/** Model typing updates the draft in place; empty Auto stays selectable until save. */ +/** An explicit model edit can re-enable a disabled draft; clearing its target keeps it off. */ export function applySidecarModelChange( override: SidecarOverride | undefined, model: string, ): SidecarOverride { - return { ...override, model }; + return { ...override, model, ...(override?.enabled !== undefined ? { enabled: model.trim().length > 0 } : {}) }; } /** * Persist shape for PUT /api/claude-code. - * Empty Auto drafts ({ backend: undefined } / blank model) become null → reload as Inherit. + * Empty Auto drafts become null → Inherit unless an explicit activation state must survive. */ export function serializeSidecarOverride( override?: SidecarOverride, ): PersistedSidecarOverride | null { if (!override) return null; const trimmed = (override.model ?? "").trim(); + const activation = override.enabled === undefined ? {} : { enabled: override.enabled }; if (!override.backend) { - if (!trimmed && override.anthropicInstance === undefined) return null; - return { backend: null, model: trimmed, + if (!trimmed && override.anthropicInstance === undefined) return override.enabled === undefined ? null : { ...activation, backend: null, model: "" }; + return { ...activation, backend: null, model: trimmed, ...(override.anthropicInstance !== undefined ? { anthropicInstance: override.anthropicInstance } : {}) }; } - return { backend: override.backend, model: trimmed, + return { ...activation, backend: override.backend, model: trimmed, ...(override.anthropicInstance !== undefined ? { anthropicInstance: override.anthropicInstance } : {}) }; } diff --git a/gui/src/pages/claude-manual-env.ts b/gui/src/pages/claude-manual-env.ts index 3c44f02c7b4..b5a2d446991 100644 --- a/gui/src/pages/claude-manual-env.ts +++ b/gui/src/pages/claude-manual-env.ts @@ -18,7 +18,7 @@ export interface AnthropicPoolOptions { available: AnthropicInstanceId[]; code?: "anthropic_helper_unavailable"; } -export interface SidecarOverride { backend?: VisionOverrideBackend; model?: string; anthropicInstance?: AnthropicInstanceId | null } +export interface SidecarOverride { enabled?: boolean; backend?: VisionOverrideBackend; model?: string; anthropicInstance?: AnthropicInstanceId | null } export interface ClaudeManualEnvState { /** diff --git a/gui/tests/claude-code-sidecar.test.ts b/gui/tests/claude-code-sidecar.test.ts index 71f450d0a4a..ee5b93943b4 100644 --- a/gui/tests/claude-code-sidecar.test.ts +++ b/gui/tests/claude-code-sidecar.test.ts @@ -32,6 +32,20 @@ test("Empty Auto serializes as null (reloads as Inherit)", () => { expect(serializeSidecarOverride({})).toBeNull(); }); +test("disabled intent survives untouched saves and backend drafts until explicit reset or model choice", () => { + const disabled = { enabled: false }; + expect(serializeSidecarOverride(disabled)).toEqual({ enabled: false, backend: null, model: "" }); + expect(serializeSidecarOverride({ enabled: true })).toEqual({ enabled: true, backend: null, model: "" }); + expect(serializeSidecarOverride(applySidecarBackendChange(disabled, "auto"))).toEqual({ enabled: false, backend: null, model: "" }); + expect(serializeSidecarOverride(applySidecarBackendChange(disabled, "inherit"))).toBeNull(); + expect(serializeSidecarOverride(applySidecarModelChange(disabled, "gpt-5.6-luna"))) + .toEqual({ enabled: true, backend: null, model: "gpt-5.6-luna" }); + const typedThenCleared = applySidecarModelChange(applySidecarModelChange(disabled, "gpt-5.6-luna"), " "); + expect(serializeSidecarOverride(typedThenCleared)).toEqual({ enabled: false, backend: null, model: "" }); + expect(serializeSidecarOverride({ enabled: false, backend: "openai", model: "gpt-5.6-luna" })) + .toEqual({ enabled: false, backend: "openai", model: "gpt-5.6-luna" }); +}); + test("Switching back to Inherit clears the override", () => { const withModel = applySidecarModelChange( applySidecarBackendChange(undefined, "auto"), diff --git a/src/config/removed-opencode-free.ts b/src/config/removed-opencode-free.ts index e308c348223..9c6c146cf24 100644 --- a/src/config/removed-opencode-free.ts +++ b/src/config/removed-opencode-free.ts @@ -95,6 +95,17 @@ export function sanitizeRemovedOpenCodeFreeForLoad(parsed: unknown): void { delete config.syncCodexSubagentDefaults; delete config.injectionEffort; } + // These sidecars default on or inherit global settings when absent. Preserve the + // off switch at both supported owners before pruning their retired model target. + for (const owner of [config, record(config.claudeCode)]) { + for (const key of ["visionSidecar", "webSearchSidecar"]) { + const sidecar = record(owner?.[key]); + if (!sidecar || !retired(sidecar.model)) continue; + sidecar.enabled = false; + delete sidecar.model; + changed = true; + } + } for (const [key, value] of Object.entries(config)) { if (key === "providers" || !/model|provider|combo|routing|claudeCode|fallback|redirect|compaction|autoReview|advisor|tokenGuardian|images|codexRoleTiers|sidecar|shadowCallIntercept/i.test(key)) continue; const next = prune(value, key === "claudeCode"); diff --git a/src/server/management/agent-settings-routes.ts b/src/server/management/agent-settings-routes.ts index df26ddc3e43..27b3857f83e 100644 --- a/src/server/management/agent-settings-routes.ts +++ b/src/server/management/agent-settings-routes.ts @@ -1516,10 +1516,10 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise visionSidecar: sidecarAnthropicPoolOptions(config, { ...helperVision, backend: helperVisionBackend }, helperParent), }, ...(webSearchOverride && Object.keys(webSearchOverride).length > 0 - ? { webSearchSidecar: { backend: webSearchOverride.backend, model: webSearchOverride.model, ...(webSearchOverride.anthropicInstance ? { anthropicInstance: webSearchOverride.anthropicInstance } : {}) } } + ? { webSearchSidecar: { enabled: webSearchOverride.enabled, backend: webSearchOverride.backend, model: webSearchOverride.model, ...(webSearchOverride.anthropicInstance ? { anthropicInstance: webSearchOverride.anthropicInstance } : {}) } } : {}), ...(visionOverride && Object.keys(visionOverride).length > 0 - ? { visionSidecar: { backend: visionOverride.backend, model: visionOverride.model, ...(visionOverride.anthropicInstance ? { anthropicInstance: visionOverride.anthropicInstance } : {}) } } + ? { visionSidecar: { enabled: visionOverride.enabled, backend: visionOverride.backend, model: visionOverride.model, ...(visionOverride.anthropicInstance ? { anthropicInstance: visionOverride.anthropicInstance } : {}) } } : {}), fastMode: config.fastMode, forceAvailable: [...visibleNativeSlugs(config), ...filterCatalogVisibleModels(models, config).map(catalogModelSlug)], @@ -1673,6 +1673,9 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise const section = body[field]; if (section === undefined || section === null) continue; if (!isPlainObject(section)) return jsonResponse({ error: `${field} must be an object or null` }, 400); + if (section.enabled !== undefined && typeof section.enabled !== "boolean") { + return jsonResponse({ error: `${field}.enabled must be a boolean` }, 400); + } // Both overrides now speak their full unions (roadmap 060 web, 170 // vision revised). Vision's third arm is "routed" (loopback through the // proxy's own router), never exa: exa is not an LLM, and accepting an @@ -1760,8 +1763,9 @@ export async function handleAgentSettingsRoutes(ctx: ManagementContext): Promise } // The per-field validation above guarantees vision only ever carries the two-member // union; the cast is the loop's shared-shape compromise, not a wider write path. - const requested = section as { backend?: "openai" | "anthropic" | "xai" | "gemini" | "exa" | null; model?: string; anthropicInstance?: AnthropicInstanceId | null }; + const requested = section as { enabled?: boolean; backend?: "openai" | "anthropic" | "xai" | "gemini" | "exa" | null; model?: string; anthropicInstance?: AnthropicInstanceId | null }; const override = { ...next[field] } as NonNullable; + if (requested.enabled !== undefined) override.enabled = requested.enabled; if (requested.anthropicInstance === null) delete override.anthropicInstance; else if (isAnthropicInstanceId(requested.anthropicInstance)) override.anthropicInstance = requested.anthropicInstance; if (requested.backend === null) delete override.backend; diff --git a/src/types/config.ts b/src/types/config.ts index 955522cf1fe..6df5a03b714 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -232,9 +232,9 @@ export interface OcxClaudeCodeConfig { /** Optional roster-style model forced on subagents at the next routed Claude Code launch. */ subagentModelForce?: string; /** Claude-originated web-search override. Unset fields inherit the global sidecar settings. */ - webSearchSidecar?: { backend?: "openai" | "anthropic" | "xai" | "gemini" | "exa"; model?: string; anthropicInstance?: AnthropicInstanceId }; + webSearchSidecar?: { enabled?: boolean; backend?: "openai" | "anthropic" | "xai" | "gemini" | "exa"; model?: string; anthropicInstance?: AnthropicInstanceId }; /** Claude-originated vision override. Unset fields inherit the global sidecar settings. */ - visionSidecar?: { backend?: "openai" | "anthropic" | "routed"; model?: string; anthropicInstance?: AnthropicInstanceId }; + visionSidecar?: { enabled?: boolean; backend?: "openai" | "anthropic" | "routed"; model?: string; anthropicInstance?: AnthropicInstanceId }; /** Persisted Claude Desktop four-family routing profile. */ desktopProfile?: OcxClaudeDesktopProfile; /** diff --git a/structure/config.md b/structure/config.md index 10271963372..c9c72dddc72 100644 --- a/structure/config.md +++ b/structure/config.md @@ -27,7 +27,7 @@ The Codex restart command follows the [CLI restart scope contract](runtime.md#cl the [source-owned credential contract](codex-home.md#orca-source-owned-account-import) governs deduplication and credential storage separately from Codex config injection. See [permanent refusal auto-pause](account-auto-pause.md). ## Config surface -`src/config/removed-opencode-free.ts` disables retired OpenCode Free provider entries (name, type or preset identity), their model references, and empty dependent combos/profiles on file load. It warns once per process about the upstream 403 gate (#7255), preserves file bytes, and restores the standard OpenAI OAuth default when the retired provider was the default. Healthy targets and unrelated settings remain intact. Retired provider and route aliases are trimmed before reference matching, including role tiers, sidecars, shadow interception and provider review selectors. Desktop assignment pruning repairs each affected family default to its first sorted remaining route or null; healthy defaults and applied markers survive. Depleted source-model allowlists disable their rule, and removing an injection model also removes its effort and native subagent sync. `src/config/live-reconcile.ts` sanitizes its authoritative raw snapshot before Claude hand-edit comparison so unrelated saves cannot resurrect removed references; healthy disk edits still win. Candidate writes retain normal validation. Custom Responses providers may declare independent boolean `supportsNativeCompactionTrigger` and `supportsNativeCompactEndpoint` capabilities in `src/config/schema/leaf-validators.ts`; absence remains off. They do not imply `decodesNativeCompactionBlobs`, official identity, or credential authority; custom native v1 key/local/forward replay enforces the separate blob capability and known serving-identity changes, while OAuth retains routed credential resolution. The native gateway contract and regression are in [Responses transport](transports/responses.md) and `tests/responses/native-compaction-gateway.test.ts`. +`src/config/removed-opencode-free.ts` disables retired OpenCode Free provider entries (name, type or preset identity), their model references, and empty dependent combos/profiles on file load. It warns once per process about the upstream 403 gate (#7255), preserves file bytes, and restores the standard OpenAI OAuth default when the retired provider was the default. Healthy targets and unrelated settings remain intact. Retired provider and route aliases are trimmed before reference matching, including role tiers, sidecars, shadow interception and provider review selectors. Desktop assignment pruning repairs each affected family default to its first sorted remaining route or null; healthy defaults and applied markers survive. Depleted source-model allowlists disable their rule, and removing an injection model also removes its effort and native subagent sync. `src/config/live-reconcile.ts` sanitizes its authoritative raw snapshot before Claude hand-edit comparison so unrelated saves cannot resurrect removed references; healthy disk edits still win. Candidate writes retain normal validation. Custom Responses providers may declare independent boolean `supportsNativeCompactionTrigger` and `supportsNativeCompactEndpoint` capabilities in `src/config/schema/leaf-validators.ts`; absence remains off. They do not imply `decodesNativeCompactionBlobs`, official identity, or credential authority; custom native v1 key/local/forward replay enforces the separate blob capability and known serving-identity changes, while OAuth retains routed credential resolution. The native gateway contract and regression are in [Responses transport](transports/responses.md) and `tests/responses/native-compaction-gateway.test.ts`. Retired model targets in global or `claudeCode` overrides of `visionSidecar` and `webSearchSidecar` leave `enabled: false` with healthy sibling settings intact, whether enabled was false, true or omitted. Healthy sidecar targets retain their configured activation state; absent sections retain the normal defaults. `src/config/schema/config-schema.ts` validates the optional `preferModelAliasIds` flag as a boolean; malformed disk values disable this raw-list projection without rejecting providers. It defaults off. Custom Responses providers can explicitly enable `preserveResponsesInputItemIds` and `preserveResponsesMessageMetadata`; both default off and accept only booleans. The [replay sanitization contract](transports/responses-wire-shapes.md) keeps ordinary destinations unchanged and preserves xAI custom-call ID repair. `src/config/schema/compaction-recovery.ts` strictly validates opt-in `compactionRecovery`; invalid disk values disable it with a warning, while candidate writes reject them. `src/config/schema/blocked-model-redirects.ts` applies the same read-degrade/write-reject boundary to malformed `blockedModelRedirects` maps. The [failure-only contract](transports/responses-failover.md) leaves provider identity, accounts and client compaction unchanged. Optional `desktopCompatibility: { startOnProxyStart: boolean }` likewise rejects invalid writes and degrades invalid disk values to off; it saves neither certificate consent nor correction mode. Its field-scoped dashboard writer adopts committed state into the live comparison baseline; see [Codex Desktop startup](clients/codex-desktop.md#proxy-startup-preference). `src/cli/config-command.ts` accepts one leading UTF-8 BOM when parsing validate/import input from a file or stdin. JSON syntax and schema validation still run before persistence; BOM characters inside string values remain data. `skills.catalog_refresh` in the proxy JSON configuration accepts `per_session` (the runtime default when absent) or `per_turn`. The former retains received skills instructions for a conversation; the latter passes through the current catalog. This is separate from Codex's `skills.include_instructions` TOML switch and does not change the live dashboard probe. See the [Responses snapshot contract](transports/responses.md#responses-httpsse). Optional `reasoningRetention` is advanced and off by default: only explicit boolean `enabled: true` activates it, so omitted/false/malformed and legacy limit-only blocks stay off. When enabled, it accepts `maxContextPercent` (>0, <=100; default 20) and `maxTokens` (positive integer; default 100000), using the smaller limit for [routed v1/v2 compaction](transports/responses-failover.md#compaction-routing-overrides). An unknown context window uses the absolute cap only. Malformed disk values degrade only this block with a value-free warning; candidate writes reject them before the load schema's catch. Absence remains unpersisted, and native/opaque protocols are unchanged. diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 47615cad546..8fd84e5881d 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -293,7 +293,7 @@ per-request first-party callback reads that live object; a failed write leaves i | Provider quotas and tests | `src/server/management/provider-routes.ts` — `GET /api/provider-quotas`, `POST /api/providers/test`, `POST /api/providers/test-completion` (explicit single-model completion diagnostic, [contract](provider-completion-diagnostic.md)), `GET/PUT /api/provider-context-caps`, `GET /api/provider-presets`. A quota read may be served from cache or force-refreshed; absent quota data is reported as unknown rather than as a measured zero. | | Models and visibility | `src/server/management/model-routes.ts` — `GET /api/models`, `PUT /api/disabled-models`, `PUT /api/model-visibility`, `PUT /api/selected-models`, `GET/POST /api/custom-models`, `PUT /api/model-settings`. Visibility writes trigger catalog sync through the owning server path. `PUT /api/model-settings` edits one routed row's capability axes in place — context window, declared input modalities, reasoning ladder and its default — through the provider-level per-model maps the runtime already reads, so the row keeps its discovery provenance instead of being replaced by a custom model. Every field is optional and `null` clears the declaration back to the registry/catalog answer; display name is deliberately absent because `PUT /api/providers/{provider}/model-display-names` already owns it. | | Effort and fallback | `src/server/management/agent-settings-routes.ts` — `GET/PUT /api/effort-caps`, `/api/subagent-models`, `/api/subagent-model-fallback`. Caps clamp; they do not reject. | -| Grok and Claude integrations | `src/server/management/agent-settings-routes.ts` — `GET /api/grok`, `PUT /api/grok/selection`, `POST /api/grok/apply`, `GET/PUT /api/claude-desktop`, `POST /api/claude-desktop/apply` (`mode`: `gateway` default for new installs, `first-party` opt-in, or legacy shapes), `GET /api/claude-desktop/status` (`mode`, `riskWarning`, `firstParty`), `GET/PUT /api/claude-code`. Gateway apply writes an external app's profile, so its status probe must read the same resolved path it writes (see [`responses.md`](transports/responses.md)); first-party apply writes only the Claude Code proxy env, see [`clients/claude-desktop.md`](clients/claude-desktop.md#desktop-modes-gateway-and-first-party). `gui/src/pages/ClaudeDesktop.tsx` renders the mode selector and sends the chosen `mode` with apply. It leads with Default model and Quick task model, which set the `opus` and `haiku` family defaults through `gui/src/pages/claude-desktop-roles.ts` (same move rules as the family lanes, which sit under a folded Advanced disclosure); the profile wire shape is unchanged. `gui/src/pages/ClaudeCode.tsx` is one page with a single Save bar; its Save body (`gui/src/pages/claude-code-save.ts`) omits `enabled`, which only the immediate connection switch writes. `PUT /api/claude-code` re-runs macOS system-env reconciliation (`src/server/system-env.ts`) whenever the body carries `systemEnv`, `authMode`, a model slot or a lever field: keys opencodex tracks as injected are refreshed or unset once the config stops producing them, and a launchd value the user set before injection is never touched. `gui/src/pages/claude-code-tier-defaults.tsx` edits `tierModels` through the Save bar's dirty/poll/revert contract. `GET /api/claude-code/model-defaults` previews local user defaults; `POST` accepts exactly a fingerprint bound to the full settings document and saved tiers (`src/claude/model-default-settings.ts`). Changed settings/tiers return 409 and existing empty/corrupt documents are refused. Apply stages private bytes, captures the actual shared file into `model-default-backups/apply-*/settings.json` under the Claude config directory, validates that capture, then publishes with an exclusive hard link. A competing destination is never replaced, even on rollback; there is a brief missing-file interval, and unsupported cross-volume/link operations have no overwrite fallback. Failed restoration returns `settings_recovery_required` with the private recovery path shown as an alert. Selected aliases receive paired `_MODEL_NAME` labels via `claudeTierDisplayName` in `src/claude/context-windows.ts`. Preview includes label additions/updates and uses an empty `after` string for removal. An existing label is managed only when it exactly matches the generated name for its current alias (an identical manually entered label is indistinguishable); custom, empty and non-string labels are preserved. Unset tiers retain both their settings model IDs and display labels; a matching generated label is removed only when an explicitly selected tier is replaced with a non-alias; no client restart, connection credential change or Desktop family edit occurs. Covered by `tests/claude-integration/claude-model-default-labels.test.ts`, `tests/claude-integration/claude-intercept-settings.test.ts`, `tests/server/model-settings-management-api.test.ts`, `gui/tests/claude-code-save.test.ts` and `gui/tests/claude-code-background-helper.test.tsx`. | +| Grok and Claude integrations | `src/server/management/agent-settings-routes.ts` — `GET /api/grok`, `PUT /api/grok/selection`, `POST /api/grok/apply`, `GET/PUT /api/claude-desktop`, `POST /api/claude-desktop/apply` (`mode`: `gateway` default for new installs, `first-party` opt-in, or legacy shapes), `GET /api/claude-desktop/status` (`mode`, `riskWarning`, `firstParty`), `GET/PUT /api/claude-code`. Gateway apply writes an external app's profile, so its status probe must read the same resolved path it writes (see [`responses.md`](transports/responses.md)); first-party apply writes only the Claude Code proxy env, see [`clients/claude-desktop.md`](clients/claude-desktop.md#desktop-modes-gateway-and-first-party). `gui/src/pages/ClaudeDesktop.tsx` renders the mode selector and sends the chosen `mode` with apply. It leads with Default model and Quick task model, which set the `opus` and `haiku` family defaults through `gui/src/pages/claude-desktop-roles.ts` (same move rules as the family lanes, which sit under a folded Advanced disclosure); the profile wire shape is unchanged. `gui/src/pages/ClaudeCode.tsx` is one page with a single Save bar; its Save body (`gui/src/pages/claude-code-save.ts`) omits the top-level `enabled`, which only the immediate connection switch writes. `PUT /api/claude-code` re-runs macOS system-env reconciliation (`src/server/system-env.ts`) whenever the body carries `systemEnv`, `authMode`, a model slot or a lever field: keys opencodex tracks as injected are refreshed or unset once the config stops producing them, and a launchd value the user set before injection is never touched. `gui/src/pages/claude-code-tier-defaults.tsx` edits `tierModels` through the Save bar's dirty/poll/revert contract. `GET /api/claude-code/model-defaults` previews local user defaults; `POST` accepts exactly a fingerprint bound to the full settings document and saved tiers (`src/claude/model-default-settings.ts`). Changed settings/tiers return 409 and existing empty/corrupt documents are refused. Apply stages private bytes, captures the actual shared file into `model-default-backups/apply-*/settings.json` under the Claude config directory, validates that capture, then publishes with an exclusive hard link. A competing destination is never replaced, even on rollback; there is a brief missing-file interval, and unsupported cross-volume/link operations have no overwrite fallback. Failed restoration returns `settings_recovery_required` with the private recovery path shown as an alert. Selected aliases receive paired `_MODEL_NAME` labels via `claudeTierDisplayName` in `src/claude/context-windows.ts`. Preview includes label additions/updates and uses an empty `after` string for removal. An existing label is managed only when it exactly matches the generated name for its current alias (an identical manually entered label is indistinguishable); custom, empty and non-string labels are preserved. Unset tiers retain both their settings model IDs and display labels; a matching generated label is removed only when an explicitly selected tier is replaced with a non-alias; no client restart, connection credential change or Desktop family edit occurs. Covered by `tests/claude-integration/claude-model-default-labels.test.ts`, `tests/claude-integration/claude-intercept-settings.test.ts`, `tests/server/model-settings-management-api.test.ts`, `gui/tests/claude-code-save.test.ts` and `gui/tests/claude-code-background-helper.test.tsx`. Claude sidecar overrides have their own optional boolean `enabled` in GET and validated PUT. `gui/src/pages/claude-code-sidecar.ts` preserves an untouched off override even without a backend/model; Inherit explicitly clears it, while selecting a nonempty model can re-enable it through the existing target validation. | The model-settings route rejects unknown fields, non-exact or reserved model IDs, and context windows outside the positive safe integers. It computes per-model maps on copies and publishes diff --git a/tests/config/config-removed-opencode-free.test.ts b/tests/config/config-removed-opencode-free.test.ts index 9e7532db0ad..6588a8c4df3 100644 --- a/tests/config/config-removed-opencode-free.test.ts +++ b/tests/config/config-removed-opencode-free.test.ts @@ -3,6 +3,10 @@ import { readFileSync, writeFileSync } from "node:fs"; import { armClaudeCodeBaseline, getConfigPath, getDefaultConfig, loadConfig, saveConfigPreservingClaudeCode } from "../../src/config"; import { readConfigDiagnostics } from "../../src/config/diagnostics"; import { PROVIDER_REGISTRY } from "../../src/providers/registry"; +import { buildClaudeReplayConfig } from "../../src/server/claude-messages"; +import { parseRequest } from "../../src/responses/parser"; +import { planVisionSidecar, shouldResolveOpenAiVisionSidecar } from "../../src/vision/plan"; +import { planWebSearch, shouldResolveOpenAiWebSearchSidecar } from "../../src/web-search"; import { createTempHome } from "../helpers/temp-home"; test("retired OpenCode Free entries and dependent routes are skipped on load, once per process", () => { @@ -179,8 +183,8 @@ test("all routed reference sites and whitespace aliases are pruned while healthy const loaded = loadConfig(); expect(readConfigDiagnostics().source).toBe("file"); expect(loaded.codexRoleTiers).toEqual({ fast: ["healthy/keep"], standard: [], frontier: [] }); - expect(loaded.visionSidecar).toBeUndefined(); - expect(loaded.webSearchSidecar).toBeUndefined(); + expect(loaded.visionSidecar).toEqual({ backend: "routed", enabled: false }); + expect(loaded.webSearchSidecar).toEqual({ backend: "openai", enabled: false }); expect(loaded.shadowCallIntercept).toEqual({ enabled: true, model: "healthy/keep", sourceModels: ["healthy/*"] }); expect(loaded.providers.healthy.defaultModel).toBeUndefined(); expect(loaded.providers.healthy.autoReviewModel).toBeUndefined(); @@ -192,6 +196,110 @@ test("all routed reference sites and whitespace aliases are pruned while healthy }); }); +test.each([false, true, undefined])("retired sidecar targets stay disabled through load, diagnostics and saves (enabled: %s)", enabled => { + const routes = { + combos: { retired: { alias: " retired-combo ", targets: [{ provider: "opencode-free", model: "old" }] } }, + routingProfiles: { retired: { alias: " retired-policy ", candidates: [{ provider: "opencode-free", model: "old" }] } }, + }; + for (const model of ["opencode-free/old", " retired-alias/old ", "combo/retired", "retired-combo", "policy/retired", "retired-policy"]) { + const settings = { ...(enabled === undefined ? {} : { enabled }), model, timeoutMs: 12_345, reasoning: "low" }; + const vision = { ...settings, backend: "routed", maxDescriptionsPerTurn: 2 }; + const search = { ...settings, backend: "openai", maxSearchesPerTurn: 3 }; + withLegacyConfig({ ...routes, visionSidecar: vision, webSearchSidecar: search }, bytes => { + const expectedVision = { backend: "routed", enabled: false, timeoutMs: 12_345, reasoning: "low", maxDescriptionsPerTurn: 2 }; + const expectedSearch = { backend: "openai", enabled: false, timeoutMs: 12_345, reasoning: "low", maxSearchesPerTurn: 3 }; + const live = loadConfig(); + const check = (config: ReturnType) => { + expect(config.visionSidecar).toEqual(expectedVision); + expect(config.webSearchSidecar).toEqual(expectedSearch); + }; + check(live); + const diagnostics = readConfigDiagnostics(); + expect(diagnostics.source).toBe("file"); + check(diagnostics.config); + expect(readFileSync(getConfigPath(), "utf8")).toBe(bytes); + armClaudeCodeBaseline(live); + for (const fastMode of [true, false]) { + live.fastMode = fastMode; + saveConfigPreservingClaudeCode(live); + check(live); + check(JSON.parse(readFileSync(getConfigPath(), "utf8"))); + check(loadConfig()); + expect(loadConfig().fastMode).toBe(fastMode); + } + }); + } +}, 60_000); + +test.each([false, true, undefined])("healthy sidecar settings keep their activation and targets (enabled: %s)", enabled => { + const settings = { ...(enabled === undefined ? {} : { enabled }), model: "healthy/keep", reasoning: "low", timeoutMs: 12_345 }; + const visionSidecar = { ...settings, backend: "routed", maxDescriptionsPerTurn: 2 }; + const webSearchSidecar = { ...settings, backend: "openai", maxSearchesPerTurn: 3 }; + withLegacyConfig({ visionSidecar, webSearchSidecar }, bytes => { + const live = loadConfig(); + expect(live.visionSidecar).toEqual(visionSidecar); + expect(live.webSearchSidecar).toEqual(webSearchSidecar); + expect(readConfigDiagnostics().config.visionSidecar).toEqual(visionSidecar); + expect(readConfigDiagnostics().config.webSearchSidecar).toEqual(webSearchSidecar); + expect(readFileSync(getConfigPath(), "utf8")).toBe(bytes); + armClaudeCodeBaseline(live); + live.fastMode = true; + saveConfigPreservingClaudeCode(live); + expect(loadConfig().visionSidecar).toEqual(visionSidecar); + expect(loadConfig().webSearchSidecar).toEqual(webSearchSidecar); + }); +}); + +test.each([false, true, undefined])("Claude retired overrides stay disabled without changing healthy globals (enabled: %s)", enabled => { + const request = parseRequest({ model: "healthy/blind", tools: [{ type: "web_search" }], input: [{ role: "user", content: [ + { type: "input_text", text: "Describe and search." }, { type: "input_image", image_url: "data:image/png;base64,aGVsbG8=" }, + ] }] }); + const globalVision = { enabled: true, backend: "routed", model: "healthy/vision", timeoutMs: 12_345 }; + const globalSearch = { enabled: true, backend: "exa", model: "healthy/search", exaApiKey: "fixture-exa-key" }; + for (const globals of [false, true]) for (const model of ["opencode-free/old", " retired-alias/old ", "combo/retired", "retired-combo", "policy/retired", "retired-policy"]) { + const activation = enabled === undefined ? {} : { enabled }; + withLegacyConfig({ + ...(globals ? { visionSidecar: globalVision, webSearchSidecar: globalSearch } : {}), + combos: { retired: { alias: " retired-combo ", targets: [{ provider: "opencode-free", model: "old" }] } }, + routingProfiles: { retired: { alias: " retired-policy ", candidates: [{ provider: "opencode-free", model: "old" }] } }, + claudeCode: { enabled: true, visionSidecar: { ...activation, backend: "routed", model }, webSearchSidecar: { ...activation, backend: "openai", model } }, + }, bytes => { + const live = loadConfig(); + live.providers.healthy!.modelInputModalities = { blind: ["text"], vision: ["text", "image"] }; + const check = (config: ReturnType) => { + expect(config.visionSidecar).toEqual(globals ? globalVision : undefined); + expect(config.webSearchSidecar).toEqual(globals ? globalSearch : undefined); + expect(config.claudeCode?.visionSidecar).toEqual({ enabled: false, backend: "routed" }); + expect(config.claudeCode?.webSearchSidecar).toEqual({ enabled: false, backend: "openai" }); + const effective = buildClaudeReplayConfig(config); + const provider = { ...config.providers.healthy!, noVisionModels: ["blind"] }; + expect(planVisionSidecar(effective, provider, "blind", request, undefined, { providerName: "healthy" })).toBeUndefined(); + expect(shouldResolveOpenAiVisionSidecar(effective, provider, "blind", request, "healthy")).toBe(false); + expect(planWebSearch(effective, request, false, provider, "blind")).toBeUndefined(); + expect(shouldResolveOpenAiWebSearchSidecar(effective, request, false)).toBe(false); + }; + check(live); + expect(readConfigDiagnostics().source).toBe("file"); + check(readConfigDiagnostics().config); + expect(readFileSync(getConfigPath(), "utf8")).toBe(bytes); + // The same legitimate global/default config remains usable outside the Claude override. + const provider = live.providers.healthy!; + expect(planVisionSidecar(live, provider, "blind", request, undefined, { providerName: "healthy" })?.backend).toBe(globals ? "routed" : undefined); + expect(planWebSearch(live, request, false, provider, "blind")?.backend).toBe(globals ? "exa" : undefined); + expect(shouldResolveOpenAiVisionSidecar(live, provider, "blind", request, "healthy")).toBe(!globals); + expect(shouldResolveOpenAiWebSearchSidecar(live, request, false)).toBe(!globals); + armClaudeCodeBaseline(live); + for (const fastMode of [true, false]) { + live.fastMode = fastMode; + saveConfigPreservingClaudeCode(live); + check(live); + check(JSON.parse(readFileSync(getConfigPath(), "utf8"))); + check(loadConfig()); + } + }); + } +}, 60_000); + test("unrelated providers and config remain unchanged", () => { const home = createTempHome("ocx-retained-provider-"); const warn = spyOn(console, "warn").mockImplementation(() => {}); diff --git a/tests/server/management-sidecar-anthropic-instance.test.ts b/tests/server/management-sidecar-anthropic-instance.test.ts index ace40532a37..b9834d5aeb3 100644 --- a/tests/server/management-sidecar-anthropic-instance.test.ts +++ b/tests/server/management-sidecar-anthropic-instance.test.ts @@ -1,9 +1,17 @@ import { afterEach, beforeEach, expect, spyOn, test } from "bun:test"; -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import * as store from "../../src/oauth/store"; import * as modelRows from "../../src/server/management/model-rows"; import * as managementShared from "../../src/server/management/shared"; -import { saveConfig } from "../../src/config"; +import { armClaudeCodeBaseline, getConfigPath, loadConfig, saveConfig } from "../../src/config"; +import * as systemEnv from "../../src/server/system-env"; +import { buildClaudeReplayConfig } from "../../src/server/claude-messages"; +import { parseRequest } from "../../src/responses/parser"; +import { planVisionSidecar } from "../../src/vision"; +import { planWebSearch } from "../../src/web-search"; +import { claudeCodeSaveBody } from "../../gui/src/pages/claude-code-save"; +import { applySidecarBackendChange, applySidecarModelChange, sidecarSelectValue } from "../../gui/src/pages/claude-code-sidecar"; +import type { ClaudeCodeState } from "../../gui/src/pages/claude-code-types"; import { anthropicOAuthInstanceConfigError, anthropicSidecarPatchError } from "../../src/config/schema/anthropic-account-pool"; import { handleManagementAPI } from "../../src/server/management-api"; import { sidecarAnthropicPoolOptions, sidecarOptionsAuth } from "../../src/server/management/web-search-sidecar-options"; @@ -53,6 +61,119 @@ function persisted(): OcxConfig { return JSON.parse(readFileSync(home.path("config.json"), "utf8")) as OcxConfig; } +test.each([false, true])("normal Claude management saves retain retired disable intent; explicit reset and model edits work (backend: %s)", async withBackend => { + let outbound = 0; + const network = spyOn(globalThis, "fetch").mockImplementation(async () => { outbound++; throw new Error("Network forbidden in settings roundtrip"); }); + const env = spyOn(systemEnv, "applySystemEnvToggle").mockResolvedValue({ reverted: false }); + try { + const global = { enabled: true, backend: "openai" as const, model: "gpt-5.6-luna" }; + const provider: OcxConfig["providers"][string] = { adapter: "openai-chat", baseUrl: "https://routed.test/v1", apiKey: "fixture-key", noVisionModels: ["blind"] }; + const forward = { providerName: "openai" as const, accountMode: "direct" as const, + provider: { adapter: "openai-responses", baseUrl: "https://chatgpt.test/v1", authMode: "forward" } as const, + authContext: { kind: "main" as const, accountId: null }, headers: new Headers({ authorization: "Bearer fixture" }) }; + const retired = { model: "opencode-free/old", ...(withBackend ? { backend: "openai" as const } : {}) }; + writeFileSync(getConfigPath(), JSON.stringify({ port: 10100, defaultProvider: "routed", providers: { routed: provider }, + webSearchSidecar: global, visionSidecar: global, claudeCode: { enabled: false, webSearchSidecar: retired, visionSidecar: retired } })); + let live = loadConfig(); + armClaudeCodeBaseline(live); + const parsed = parseRequest({ model: "routed/blind", tools: [{ type: "web_search" }], input: [{ role: "user", content: [ + { type: "input_text", text: "Search and inspect." }, { type: "input_image", image_url: "data:image/png;base64,aGVsbG8=" }, + ] }] }); + const checkPlans = (config: OcxConfig, enabled: boolean) => { + const effective = buildClaudeReplayConfig(config); + expect(!!planVisionSidecar(effective, provider, "blind", parsed, forward)).toBe(enabled); + expect(!!planWebSearch(effective, parsed, false, provider, "blind", forward)).toBe(enabled); + expect(config.visionSidecar).toEqual(global); + expect(config.webSearchSidecar).toEqual(global); + }; + for (const fastMode of [true, false]) { + const state = await (await request(live, "/api/claude-code")).json() as ClaudeCodeState; + const body = claudeCodeSaveBody({ ...state, fastMode }, []); + expect((await request(live, "/api/claude-code", body)).status).toBe(200); + live = loadConfig(); + checkPlans(live, false); + expect(live.claudeCode?.visionSidecar?.enabled).toBe(false); + expect(live.claudeCode?.webSearchSidecar?.enabled).toBe(false); + expect(live.fastMode).toBe(fastMode); + } + const state = await (await request(live, "/api/claude-code")).json() as ClaudeCodeState; + const inherit = claudeCodeSaveBody({ ...state, + visionSidecar: applySidecarBackendChange(state.visionSidecar, "inherit"), + webSearchSidecar: applySidecarBackendChange(state.webSearchSidecar, "inherit"), + }, []); + expect((await request(live, "/api/claude-code", inherit)).status).toBe(200); + live = loadConfig(); + expect(live.claudeCode?.visionSidecar).toBeUndefined(); + expect(live.claudeCode?.webSearchSidecar).toBeUndefined(); + checkPlans(live, true); + expect((await request(live, "/api/claude-code", { visionSidecar: { enabled: false }, webSearchSidecar: { enabled: false } })).status).toBe(200); + const disabled = await (await request(live, "/api/claude-code")).json() as ClaudeCodeState; + const beforeInvalidEdit = readFileSync(getConfigPath(), "utf8"); + for (const field of ["visionSidecar", "webSearchSidecar"] as const) { + const invalid = claudeCodeSaveBody({ ...disabled, [field]: applySidecarModelChange(disabled[field], + field === "visionSidecar" ? "routed/blind" : "not-a-search-candidate") }, []); + expect((await request(live, "/api/claude-code", invalid)).status).toBe(400); + expect(readFileSync(getConfigPath(), "utf8")).toBe(beforeInvalidEdit); + expect(loadConfig().claudeCode?.[field]?.enabled).toBe(false); + } + const reenable = claudeCodeSaveBody({ ...disabled, + visionSidecar: applySidecarModelChange(disabled.visionSidecar, "gpt-5.6-luna"), + webSearchSidecar: applySidecarModelChange(disabled.webSearchSidecar, "gpt-5.6-luna"), + }, []); + expect((await request(live, "/api/claude-code", reenable)).status).toBe(200); + live = loadConfig(); + expect(live.claudeCode?.visionSidecar?.enabled).toBe(true); + expect(live.claudeCode?.webSearchSidecar?.enabled).toBe(true); + checkPlans(live, true); + expect(outbound).toBe(0); + } finally { env.mockRestore(); network.mockRestore(); } +}, 60_000); + +test("Claude sidecar enabled writes reject non-booleans without changing config", async () => { + const config = anthropicInstanceConfig(); + saveConfig(config); + const bytes = readFileSync(getConfigPath(), "utf8"); + for (const field of ["visionSidecar", "webSearchSidecar"]) for (const enabled of [null, 0, "false", {}, []]) { + expect((await request(config, "/api/claude-code", { [field]: { enabled } })).status).toBe(400); + expect(readFileSync(getConfigPath(), "utf8")).toBe(bytes); + expect(config.claudeCode?.[field as "visionSidecar"]).toBeUndefined(); + } +}); + +test.each(["backend", "model", "both"] as const)("Claude Auto edits clear persisted %s while retaining activation intent", async scenario => { + let outbound = 0; + const network = spyOn(globalThis, "fetch").mockImplementation(async () => { outbound++; throw new Error("Network forbidden in Auto roundtrip"); }); + const env = spyOn(systemEnv, "applySystemEnvToggle").mockResolvedValue({ reverted: false }); + try { + const global = { enabled: true, backend: "openai" as const, model: "gpt-5.6-luna" }; + for (const field of ["visionSidecar", "webSearchSidecar"] as const) { + const stored = { enabled: scenario !== "backend", + ...(scenario === "model" ? {} : { backend: field === "visionSidecar" ? "routed" : "anthropic" }), + ...(scenario === "backend" ? {} : { model: scenario === "both" ? (field === "visionSidecar" ? "routed/vision" : "claude-haiku-4-5") : "gpt-5.6-luna" }), + }; + writeFileSync(getConfigPath(), JSON.stringify({ port: 10100, defaultProvider: "routed", providers: { + routed: { adapter: "openai-chat", baseUrl: "https://routed.test/v1", apiKey: "fixture-key" }, + }, webSearchSidecar: global, visionSidecar: global, claudeCode: { enabled: false, [field]: stored } })); + const live = loadConfig(); + armClaudeCodeBaseline(live); + const state = await (await request(live, "/api/claude-code")).json() as ClaudeCodeState; + let draft = applySidecarBackendChange(state[field], "auto")!; + if (scenario !== "backend") draft = applySidecarModelChange(draft, ""); + const body = claudeCodeSaveBody({ ...state, [field]: draft, fastMode: true }, []); + expect((await request(live, "/api/claude-code", body)).status).toBe(200); + const reloaded = loadConfig(); + expect(reloaded.claudeCode?.[field]).toEqual({ enabled: false }); + expect(reloaded.visionSidecar).toEqual(global); + expect(reloaded.webSearchSidecar).toEqual(global); + const after = await (await request(reloaded, "/api/claude-code")).json() as ClaudeCodeState; + expect(sidecarSelectValue(after[field])).toBe("auto"); + expect(after[field]?.model).toBeUndefined(); + expect(reloaded.fastMode).toBe(true); + } + expect(outbound).toBe(0); + } finally { env.mockRestore(); network.mockRestore(); } +}, 60_000); + for (const instance of ["anthropic", "anthropic2"] as const) { test(`global web-search and vision round-trip ${instance}; omission preserves and null deletes`, async () => { const config = anthropicInstanceConfig(); diff --git a/tests/vision/mistral-vision-sidecar.test.ts b/tests/vision/mistral-vision-sidecar.test.ts index d6883474ad3..fc754916f74 100644 --- a/tests/vision/mistral-vision-sidecar.test.ts +++ b/tests/vision/mistral-vision-sidecar.test.ts @@ -1,10 +1,11 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync } from "node:fs"; +import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { saveConfig } from "../../src/config"; +import { getConfigPath, saveConfig } from "../../src/config"; import { buildCatalogEntries, gatherRoutedModels } from "../../src/codex/catalog"; import { clearModelCache } from "../../src/codex/model-cache"; +import { settleBackgroundWork } from "../../src/lib/background-work"; import { routeModel } from "../../src/router"; import { startServer } from "../../src/server"; import type { OcxConfig } from "../../src/types"; @@ -13,7 +14,7 @@ import { requiresVisionPreprocessing } from "../../src/vision/plan"; import { fakeChatGptJwt } from "../helpers/fake-chatgpt-jwt"; import { withStubbedProviderFetch } from "../helpers/catalog-provider-fetch"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; -import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { removeTreeAfterBackgroundWork } from "../helpers/remove-tree"; const GLM = "zai-glm-5-3"; const NATIVE = "mistral-large-4-0"; @@ -25,6 +26,11 @@ let home = ""; let codexHome: IsolatedCodexHome; let sidecarCalls: Array>; let mainCalls: Array>; +let proxy: ReturnType | undefined; +let requestFlight: Promise<{ status: number; body: string }> | undefined; +let cleanupFlight: Promise | undefined; +let sidecarWait: Promise | undefined; +let sidecarEntered: (() => void) | undefined; beforeEach(() => { previousHome = process.env.OPENCODEX_HOME; @@ -35,6 +41,10 @@ beforeEach(() => { resetVisionDescriptionCache(); sidecarCalls = []; mainCalls = []; + proxy = undefined; + requestFlight = cleanupFlight = undefined; + sidecarWait = undefined; + sidecarEntered = undefined; globalThis.fetch = (async (input, init) => { const url = new URL(input instanceof Request ? input.url : String(input)); if (url.origin === "https://api.mistral.ai") { @@ -53,6 +63,8 @@ beforeEach(() => { } if (url.origin === "https://chatgpt.com" && url.pathname === "/backend-api/codex/responses") { sidecarCalls.push(JSON.parse(String(init?.body))); + sidecarEntered?.(); + await sidecarWait; return new Response(`data: ${JSON.stringify({ type: "response.output_text.delta", delta: CAPTION })}\n\ndata: [DONE]\n\n`, { headers: { "content-type": "text/event-stream" }, }); @@ -62,15 +74,37 @@ beforeEach(() => { }) as typeof fetch; }); -afterEach(() => { - globalThis.fetch = originalFetch; - clearModelCache(); - resetVisionDescriptionCache(); - if (previousHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = previousHome; - codexHome.restore(); - removeTreeWithRetry(home); -}); +function finishFixture(): Promise { + return cleanupFlight ??= (async () => { + // Background settlement alone does not own HTTP requests. Close ingress gracefully + // and await the complete response body before any fetch mock or home is restored. + await proxy?.stop(false); + await requestFlight?.catch(() => undefined); + await removeTreeAfterBackgroundWork(home); + clearModelCache(); + resetVisionDescriptionCache(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + codexHome.restore(); + globalThis.fetch = originalFetch; + })(); +} + +afterEach(finishFixture, 60_000); + +function imageRequest(model: string): Promise<{ status: number; body: string }> { + return requestFlight = (async () => { + const token = fakeChatGptJwt({ chatgpt_account_id: "acct-mistral-vision-fixture" }); + const response = await fetch(new URL("/v1/responses", proxy!.url), { + method: "POST", + headers: { "content-type": "application/json", authorization: `Bearer ${token}`, "chatgpt-account-id": "acct-mistral-vision-fixture" }, + body: JSON.stringify({ model, stream: false, input: [{ role: "user", content: [ + { type: "input_text", text: "Describe this image." }, { type: "input_image", image_url: IMAGE }, + ] }] }), + }); + return { status: response.status, body: await response.text() }; + })(); +} /** Build an isolated Mistral configuration with a caller-authenticated OpenAI vision helper. */ function configuration(): OcxConfig { @@ -132,43 +166,60 @@ describe("Mistral GLM vision sidecar defaults", () => { } }); - test.each(["sidecar", "disabled", "native"] as const)("image request uses the %s path", async mode => { + test.each(["sidecar", "disabled", "native", "retired-disabled", "retired-enabled", "retired-implicit"] as const)("image request uses the %s path", async mode => { const config = configuration(); if (mode === "disabled") config.visionSidecar = { enabled: false }; saveConfig(config); - const server = startServer(0); - try { - const token = fakeChatGptJwt({ chatgpt_account_id: "acct-mistral-vision-fixture" }); - const response = await fetch(new URL("/v1/responses", server.url), { - method: "POST", - headers: { - "content-type": "application/json", authorization: `Bearer ${token}`, - "chatgpt-account-id": "acct-mistral-vision-fixture", - }, - body: JSON.stringify({ model: `mistral/${mode === "native" ? NATIVE : GLM}`, stream: false, - input: [{ role: "user", content: [ - { type: "input_text", text: "Describe this image." }, { type: "input_image", image_url: IMAGE }, - ] }], - }), - }); - expect(response.status).toBe(200); - await response.text(); - expect(mainCalls).toHaveLength(1); - expect(sidecarCalls).toHaveLength(mode === "sidecar" ? 1 : 0); - const forwarded = JSON.stringify(mainCalls[0]); - if (mode === "sidecar") { - expect(sidecarCalls[0]!.model).toBe("gpt-5.6-luna"); - expect(JSON.stringify(sidecarCalls[0])).toContain(IMAGE); - expect(forwarded).toContain(CAPTION); - } - if (mode === "native") expect(forwarded).toContain(IMAGE); - else { - expect(forwarded).not.toContain(IMAGE); - expect(forwarded).not.toContain("image_url"); - } - if (mode === "disabled") expect(forwarded).toContain("[image omitted:"); - } finally { - await server.stop(true); + if (mode.startsWith("retired-")) { + // Exercise file loading before real request admission, with a healthy helper available. + writeFileSync(getConfigPath(), JSON.stringify({ ...config, visionSidecar: { + backend: "routed", model: "opencode-free/old", + ...(mode === "retired-implicit" ? {} : { enabled: mode === "retired-enabled" }), + } })); } - }); + proxy = startServer(0); + const response = await imageRequest(`mistral/${mode === "native" ? NATIVE : GLM}`); + expect(response.status).toBe(200); + expect(mainCalls).toHaveLength(1); + expect(sidecarCalls).toHaveLength(mode === "sidecar" ? 1 : 0); + const forwarded = JSON.stringify(mainCalls[0]); + if (mode === "sidecar") { + expect(sidecarCalls[0]!.model).toBe("gpt-5.6-luna"); + expect(JSON.stringify(sidecarCalls[0])).toContain(IMAGE); + expect(forwarded).toContain(CAPTION); + } + if (mode === "native") expect(forwarded).toContain(IMAGE); + else { + expect(forwarded).not.toContain(IMAGE); + expect(forwarded).not.toContain("image_url"); + } + if (mode === "disabled" || mode.startsWith("retired-")) expect(forwarded).toContain("[image omitted:"); + }, 60_000); + + test("fixture retains its fetch mock until an in-flight request and server have finished", async () => { + let release!: () => void; + sidecarWait = new Promise(resolve => { release = resolve; }); + const entered = new Promise(resolve => { sidecarEntered = resolve; }); + saveConfig(configuration()); + proxy = startServer(0); + const guardedFetch = globalThis.fetch; + let requestFinished = false; + const response = imageRequest(`mistral/${GLM}`).then(value => { requestFinished = true; return value; }); + try { + await entered; + expect((await settleBackgroundWork({ timeoutMs: 5_000 })).settled).toBe(true); + expect(requestFinished).toBe(false); + let cleanupFinished = false; + const cleanup = finishFixture().then(() => { cleanupFinished = true; }); + await Promise.resolve(); + expect(cleanupFinished).toBe(false); + expect(globalThis.fetch).toBe(guardedFetch); + release(); + await cleanup; + expect((await response).status).toBe(200); + expect(requestFinished).toBe(true); + expect(globalThis.fetch).toBe(originalFetch); + expect(sidecarCalls).toHaveLength(1); + } finally { release(); } + }, 60_000); }); From 59582c744b64f69f197eb3712dca9374364eb02c Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:09:10 +0900 Subject: [PATCH 2/7] fix(sidecar): preserve activation through model edits and drain test fixtures --- gui/src/pages/claude-code-save.ts | 11 +- gui/src/pages/claude-code-sidecar.ts | 19 +- gui/tests/claude-code-save.test.ts | 21 ++ gui/tests/claude-code-sidecar.test.ts | 12 + structure/gui-and-management-api.md | 2 +- ...agement-sidecar-anthropic-instance.test.ts | 10 +- tests/vision/mistral-vision-sidecar.test.ts | 300 ++++++++++++++++-- 7 files changed, 346 insertions(+), 29 deletions(-) diff --git a/gui/src/pages/claude-code-save.ts b/gui/src/pages/claude-code-save.ts index 58e938502f2..8196de3196e 100644 --- a/gui/src/pages/claude-code-save.ts +++ b/gui/src/pages/claude-code-save.ts @@ -1,4 +1,4 @@ -import { serializeSidecarOverride } from "./claude-code-sidecar"; +import { confirmSidecarOverride, serializeSidecarOverride } from "./claude-code-sidecar"; import type { ClaudeCodeState, MapRow } from "./claude-code-types"; import { newClientId } from "./claude-code-types"; @@ -118,7 +118,10 @@ export function applyServerRead(current: ClaudeCodeEditState | null, next: Claud */ export function savedCopy(serverState: ClaudeCodeState, submitted: ClaudeCodeEditable): ClaudeCodeEditable { return { - state: { ...serverState, ...pickEditable(submitted.state) }, + state: { ...serverState, ...pickEditable(submitted.state), + visionSidecar: confirmSidecarOverride(submitted.state.visionSidecar), + webSearchSidecar: confirmSidecarOverride(submitted.state.webSearchSidecar), + }, rows: Object.entries(normalizedModelMap(submitted.rows)).map(([from, to]) => ({ id: newClientId(), from, to })), }; } @@ -137,5 +140,7 @@ export function acknowledgeSave(current: ClaudeCodeEditState | null, submitted: const rows = baseline.rows; const unchanged = claudeCodeDraftKey(current.draft) === claudeCodeDraftKey(submitted); // Untouched since submission: show what was stored. Edited meanwhile: keep the edits, dirty. - return { draft: unchanged ? { state: current.draft.state, rows } : current.draft, baseline, adoptNextRead: unchanged }; + return { draft: unchanged ? { state: { ...current.draft.state, + visionSidecar: baseline.state.visionSidecar, webSearchSidecar: baseline.state.webSearchSidecar, + }, rows } : current.draft, baseline, adoptNextRead: unchanged }; } diff --git a/gui/src/pages/claude-code-sidecar.ts b/gui/src/pages/claude-code-sidecar.ts index 156f4e686c8..36407ed8cac 100644 --- a/gui/src/pages/claude-code-sidecar.ts +++ b/gui/src/pages/claude-code-sidecar.ts @@ -8,6 +8,18 @@ import type { AnthropicInstanceId, SidecarOverride, VisionOverrideBackend } from export type SidecarSelectValue = "inherit" | "auto" | VisionOverrideBackend; +// Draft-only provenance follows object spreads but cannot enter JSON or the PUT shape. +const modelEditStartedDisabled = Symbol("modelEditStartedDisabled"); +type SidecarDraft = SidecarOverride & { [modelEditStartedDisabled]?: true }; + +/** A confirmed save starts a new editing baseline. */ +export function confirmSidecarOverride(override?: SidecarOverride): SidecarOverride | undefined { + if (!override) return undefined; + const next = { ...override } as SidecarDraft; + delete next[modelEditStartedDisabled]; + return next; +} + export type PersistedSidecarOverride = { enabled?: boolean; backend?: VisionOverrideBackend | null; @@ -39,12 +51,15 @@ export function applySidecarPoolChange(override: SidecarOverride | undefined, va return next; } -/** An explicit model edit can re-enable a disabled draft; clearing its target keeps it off. */ +/** Model edits preserve activation unless this draft started disabled. */ export function applySidecarModelChange( override: SidecarOverride | undefined, model: string, ): SidecarOverride { - return { ...override, model, ...(override?.enabled !== undefined ? { enabled: model.trim().length > 0 } : {}) }; + const startedDisabled = override?.enabled === false || (override as SidecarDraft | undefined)?.[modelEditStartedDisabled]; + return { ...override, model, ...(startedDisabled ? { + enabled: model.trim().length > 0, [modelEditStartedDisabled]: true as const, + } : {}) }; } /** diff --git a/gui/tests/claude-code-save.test.ts b/gui/tests/claude-code-save.test.ts index 1c71930f306..b40ac3e058d 100644 --- a/gui/tests/claude-code-save.test.ts +++ b/gui/tests/claude-code-save.test.ts @@ -10,6 +10,7 @@ import { type ClaudeCodeEditable, } from "../src/pages/claude-code-save"; import { normalizeContextAccounting, type ClaudeCodeState, type MapRow } from "../src/pages/claude-code-types"; +import { applySidecarBackendChange, applySidecarModelChange, applySidecarPoolChange } from "../src/pages/claude-code-sidecar"; const STATE = { enabled: true, @@ -38,6 +39,26 @@ const STATE = { const row = (from: string, to: string, id = `${from}->${to}`): MapRow => ({ id, from, to }); const editable = (state: Partial = {}, rows: MapRow[] = []): ClaudeCodeEditable => ({ state: { ...STATE, ...state }, rows }); +test("model-edit activation provenance survives drafts but ends at a confirmed save or revert", () => { + const base = editable({ visionSidecar: { enabled: false } }); + const typed = applySidecarModelChange(base.state.visionSidecar, "gpt-5.6-luna"); + const submitted = editable({ visionSidecar: applySidecarPoolChange(applySidecarBackendChange(typed, "openai"), "") }); + const body = claudeCodeSaveBody(submitted.state, []); + expect(body.visionSidecar).toEqual({ enabled: true, backend: "openai", model: "gpt-5.6-luna" }); + expect(Object.getOwnPropertySymbols(body.visionSidecar!)).toHaveLength(0); + expect(JSON.parse(JSON.stringify(body)).visionSidecar).toEqual(body.visionSidecar); + const polled = applyServerRead({ draft: submitted, baseline: base, adoptNextRead: false }, base); + expect(applySidecarModelChange(polled.draft.state.visionSidecar, "").enabled).toBe(false); + expect(revertEditable(submitted.state, base.state).visionSidecar).toEqual({ enabled: false }); + const acked = acknowledgeSave({ draft: submitted, baseline: base, adoptNextRead: false }, submitted)!; + // Even if the acknowledging GET fails, the successful PUT is the new activation baseline. + expect(applySidecarModelChange(acked.draft.state.visionSidecar, "").enabled).toBe(true); + const reverted = revertEditable(base.state, acked.baseline.state); + expect(applySidecarModelChange(reverted.visionSidecar, "").enabled).toBe(true); + const fresh = editable({ visionSidecar: { enabled: true, backend: "openai", model: "gpt-5.6-luna" } }); + expect(applySidecarModelChange(applyServerRead(acked, fresh).draft.state.visionSidecar, "").enabled).toBe(true); +}); + test("tier defaults participate in save, polling preservation and revert", () => { const tiers = { opus: "combo/opus", sonnet: "combo/sonnet", haiku: "combo/haiku" }; const draft = editable({ tierModels: tiers }); diff --git a/gui/tests/claude-code-sidecar.test.ts b/gui/tests/claude-code-sidecar.test.ts index ee5b93943b4..4059f47d527 100644 --- a/gui/tests/claude-code-sidecar.test.ts +++ b/gui/tests/claude-code-sidecar.test.ts @@ -56,6 +56,18 @@ test("Switching back to Inherit clears the override", () => { expect(serializeSidecarOverride(undefined)).toBeNull(); }); +test("clearing a model preserves an already-enabled or implicit override", () => { + for (const enabled of [true, undefined]) { + const initial = { enabled, backend: "openai" as const, model: "gpt-5.6-luna" }; + const cleared = applySidecarModelChange(initial, " "); + expect(serializeSidecarOverride(cleared)).toEqual({ + ...(enabled === undefined ? {} : { enabled }), backend: "openai", model: "", + }); + expect(serializeSidecarOverride(applySidecarModelChange(applySidecarModelChange(initial, "replacement"), ""))) + .toEqual(serializeSidecarOverride(cleared)); + } +}); + test("Switching Auto ↔ OpenAI ↔ Anthropic preserves the model", () => { let override = applySidecarModelChange( applySidecarBackendChange(undefined, "auto"), diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 8fd84e5881d..268c9c0d110 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -293,7 +293,7 @@ per-request first-party callback reads that live object; a failed write leaves i | Provider quotas and tests | `src/server/management/provider-routes.ts` — `GET /api/provider-quotas`, `POST /api/providers/test`, `POST /api/providers/test-completion` (explicit single-model completion diagnostic, [contract](provider-completion-diagnostic.md)), `GET/PUT /api/provider-context-caps`, `GET /api/provider-presets`. A quota read may be served from cache or force-refreshed; absent quota data is reported as unknown rather than as a measured zero. | | Models and visibility | `src/server/management/model-routes.ts` — `GET /api/models`, `PUT /api/disabled-models`, `PUT /api/model-visibility`, `PUT /api/selected-models`, `GET/POST /api/custom-models`, `PUT /api/model-settings`. Visibility writes trigger catalog sync through the owning server path. `PUT /api/model-settings` edits one routed row's capability axes in place — context window, declared input modalities, reasoning ladder and its default — through the provider-level per-model maps the runtime already reads, so the row keeps its discovery provenance instead of being replaced by a custom model. Every field is optional and `null` clears the declaration back to the registry/catalog answer; display name is deliberately absent because `PUT /api/providers/{provider}/model-display-names` already owns it. | | Effort and fallback | `src/server/management/agent-settings-routes.ts` — `GET/PUT /api/effort-caps`, `/api/subagent-models`, `/api/subagent-model-fallback`. Caps clamp; they do not reject. | -| Grok and Claude integrations | `src/server/management/agent-settings-routes.ts` — `GET /api/grok`, `PUT /api/grok/selection`, `POST /api/grok/apply`, `GET/PUT /api/claude-desktop`, `POST /api/claude-desktop/apply` (`mode`: `gateway` default for new installs, `first-party` opt-in, or legacy shapes), `GET /api/claude-desktop/status` (`mode`, `riskWarning`, `firstParty`), `GET/PUT /api/claude-code`. Gateway apply writes an external app's profile, so its status probe must read the same resolved path it writes (see [`responses.md`](transports/responses.md)); first-party apply writes only the Claude Code proxy env, see [`clients/claude-desktop.md`](clients/claude-desktop.md#desktop-modes-gateway-and-first-party). `gui/src/pages/ClaudeDesktop.tsx` renders the mode selector and sends the chosen `mode` with apply. It leads with Default model and Quick task model, which set the `opus` and `haiku` family defaults through `gui/src/pages/claude-desktop-roles.ts` (same move rules as the family lanes, which sit under a folded Advanced disclosure); the profile wire shape is unchanged. `gui/src/pages/ClaudeCode.tsx` is one page with a single Save bar; its Save body (`gui/src/pages/claude-code-save.ts`) omits the top-level `enabled`, which only the immediate connection switch writes. `PUT /api/claude-code` re-runs macOS system-env reconciliation (`src/server/system-env.ts`) whenever the body carries `systemEnv`, `authMode`, a model slot or a lever field: keys opencodex tracks as injected are refreshed or unset once the config stops producing them, and a launchd value the user set before injection is never touched. `gui/src/pages/claude-code-tier-defaults.tsx` edits `tierModels` through the Save bar's dirty/poll/revert contract. `GET /api/claude-code/model-defaults` previews local user defaults; `POST` accepts exactly a fingerprint bound to the full settings document and saved tiers (`src/claude/model-default-settings.ts`). Changed settings/tiers return 409 and existing empty/corrupt documents are refused. Apply stages private bytes, captures the actual shared file into `model-default-backups/apply-*/settings.json` under the Claude config directory, validates that capture, then publishes with an exclusive hard link. A competing destination is never replaced, even on rollback; there is a brief missing-file interval, and unsupported cross-volume/link operations have no overwrite fallback. Failed restoration returns `settings_recovery_required` with the private recovery path shown as an alert. Selected aliases receive paired `_MODEL_NAME` labels via `claudeTierDisplayName` in `src/claude/context-windows.ts`. Preview includes label additions/updates and uses an empty `after` string for removal. An existing label is managed only when it exactly matches the generated name for its current alias (an identical manually entered label is indistinguishable); custom, empty and non-string labels are preserved. Unset tiers retain both their settings model IDs and display labels; a matching generated label is removed only when an explicitly selected tier is replaced with a non-alias; no client restart, connection credential change or Desktop family edit occurs. Covered by `tests/claude-integration/claude-model-default-labels.test.ts`, `tests/claude-integration/claude-intercept-settings.test.ts`, `tests/server/model-settings-management-api.test.ts`, `gui/tests/claude-code-save.test.ts` and `gui/tests/claude-code-background-helper.test.tsx`. Claude sidecar overrides have their own optional boolean `enabled` in GET and validated PUT. `gui/src/pages/claude-code-sidecar.ts` preserves an untouched off override even without a backend/model; Inherit explicitly clears it, while selecting a nonempty model can re-enable it through the existing target validation. | +| Grok and Claude integrations | `src/server/management/agent-settings-routes.ts` — `GET /api/grok`, `PUT /api/grok/selection`, `POST /api/grok/apply`, `GET/PUT /api/claude-desktop`, `POST /api/claude-desktop/apply` (`mode`: `gateway` default for new installs, `first-party` opt-in, or legacy shapes), `GET /api/claude-desktop/status` (`mode`, `riskWarning`, `firstParty`), `GET/PUT /api/claude-code`. Gateway apply writes an external app's profile, so its status probe must read the same resolved path it writes (see [`responses.md`](transports/responses.md)); first-party apply writes only the Claude Code proxy env, see [`clients/claude-desktop.md`](clients/claude-desktop.md#desktop-modes-gateway-and-first-party). `gui/src/pages/ClaudeDesktop.tsx` renders the mode selector and sends the chosen `mode` with apply. It leads with Default model and Quick task model, which set the `opus` and `haiku` family defaults through `gui/src/pages/claude-desktop-roles.ts` (same move rules as the family lanes, which sit under a folded Advanced disclosure); the profile wire shape is unchanged. `gui/src/pages/ClaudeCode.tsx` is one page with a single Save bar; its Save body (`gui/src/pages/claude-code-save.ts`) omits the top-level `enabled`, which only the immediate connection switch writes. `PUT /api/claude-code` re-runs macOS system-env reconciliation (`src/server/system-env.ts`) whenever the body carries `systemEnv`, `authMode`, a model slot or a lever field: keys opencodex tracks as injected are refreshed or unset once the config stops producing them, and a launchd value the user set before injection is never touched. `gui/src/pages/claude-code-tier-defaults.tsx` edits `tierModels` through the Save bar's dirty/poll/revert contract. `GET /api/claude-code/model-defaults` previews local user defaults; `POST` accepts exactly a fingerprint bound to the full settings document and saved tiers (`src/claude/model-default-settings.ts`). Changed settings/tiers return 409 and existing empty/corrupt documents are refused. Apply stages private bytes, captures the actual shared file into `model-default-backups/apply-*/settings.json` under the Claude config directory, validates that capture, then publishes with an exclusive hard link. A competing destination is never replaced, even on rollback; there is a brief missing-file interval, and unsupported cross-volume/link operations have no overwrite fallback. Failed restoration returns `settings_recovery_required` with the private recovery path shown as an alert. Selected aliases receive paired `_MODEL_NAME` labels via `claudeTierDisplayName` in `src/claude/context-windows.ts`. Preview includes label additions/updates and uses an empty `after` string for removal. An existing label is managed only when it exactly matches the generated name for its current alias (an identical manually entered label is indistinguishable); custom, empty and non-string labels are preserved. Unset tiers retain both their settings model IDs and display labels; a matching generated label is removed only when an explicitly selected tier is replaced with a non-alias; no client restart, connection credential change or Desktop family edit occurs. Covered by `tests/claude-integration/claude-model-default-labels.test.ts`, `tests/claude-integration/claude-intercept-settings.test.ts`, `tests/server/model-settings-management-api.test.ts`, `gui/tests/claude-code-save.test.ts` and `gui/tests/claude-code-background-helper.test.tsx`. Claude sidecar overrides have their own optional boolean `enabled` in GET and validated PUT. `gui/src/pages/claude-code-sidecar.ts` preserves an untouched off override even without a backend/model; Inherit explicitly clears it, while selecting a nonempty model can re-enable it through the existing target validation. Model edits preserve an already enabled override; typing then clearing a model in an off draft returns it to off. Draft provenance never enters PUT or confirmed saved state. | The model-settings route rejects unknown fields, non-exact or reserved model IDs, and context windows outside the positive safe integers. It computes per-model maps on copies and publishes diff --git a/tests/server/management-sidecar-anthropic-instance.test.ts b/tests/server/management-sidecar-anthropic-instance.test.ts index b9834d5aeb3..25b5c4c54e1 100644 --- a/tests/server/management-sidecar-anthropic-instance.test.ts +++ b/tests/server/management-sidecar-anthropic-instance.test.ts @@ -108,6 +108,13 @@ test.each([false, true])("normal Claude management saves retain retired disable checkPlans(live, true); expect((await request(live, "/api/claude-code", { visionSidecar: { enabled: false }, webSearchSidecar: { enabled: false } })).status).toBe(200); const disabled = await (await request(live, "/api/claude-code")).json() as ClaudeCodeState; + const abandoned = claudeCodeSaveBody({ ...disabled, + visionSidecar: applySidecarModelChange(applySidecarModelChange(disabled.visionSidecar, "gpt-5.6-luna"), ""), + webSearchSidecar: applySidecarModelChange(applySidecarModelChange(disabled.webSearchSidecar, "gpt-5.6-luna"), ""), + }, []); + expect((await request(live, "/api/claude-code", abandoned)).status).toBe(200); + live = loadConfig(); + checkPlans(live, false); const beforeInvalidEdit = readFileSync(getConfigPath(), "utf8"); for (const field of ["visionSidecar", "webSearchSidecar"] as const) { const invalid = claudeCodeSaveBody({ ...disabled, [field]: applySidecarModelChange(disabled[field], @@ -162,7 +169,8 @@ test.each(["backend", "model", "both"] as const)("Claude Auto edits clear persis const body = claudeCodeSaveBody({ ...state, [field]: draft, fastMode: true }, []); expect((await request(live, "/api/claude-code", body)).status).toBe(200); const reloaded = loadConfig(); - expect(reloaded.claudeCode?.[field]).toEqual({ enabled: false }); + expect(reloaded.claudeCode?.[field]).toEqual({ enabled: stored.enabled }); + expect(Object.getOwnPropertySymbols(body[field]!)).toHaveLength(0); expect(reloaded.visionSidecar).toEqual(global); expect(reloaded.webSearchSidecar).toEqual(global); const after = await (await request(reloaded, "/api/claude-code")).json() as ClaudeCodeState; diff --git a/tests/vision/mistral-vision-sidecar.test.ts b/tests/vision/mistral-vision-sidecar.test.ts index fc754916f74..e2b0582fe18 100644 --- a/tests/vision/mistral-vision-sidecar.test.ts +++ b/tests/vision/mistral-vision-sidecar.test.ts @@ -1,38 +1,86 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { getConfigPath, saveConfig } from "../../src/config"; +import { getConfigPath, loadConfig, saveConfig } from "../../src/config"; import { buildCatalogEntries, gatherRoutedModels } from "../../src/codex/catalog"; import { clearModelCache } from "../../src/codex/model-cache"; -import { settleBackgroundWork } from "../../src/lib/background-work"; +import { pendingBackgroundWork, settleBackgroundWork, trackBackgroundWork } from "../../src/lib/background-work"; +import { flushWindowsSecretAclReapsBeforeRemoval } from "../../src/lib/windows-secret-acl"; +import * as outbound from "../../src/lib/provider-outbound"; import { routeModel } from "../../src/router"; import { startServer } from "../../src/server"; import type { OcxConfig } from "../../src/types"; import { resetVisionDescriptionCache } from "../../src/vision"; import { requiresVisionPreprocessing } from "../../src/vision/plan"; import { fakeChatGptJwt } from "../helpers/fake-chatgpt-jwt"; -import { withStubbedProviderFetch } from "../helpers/catalog-provider-fetch"; +import { stubbedProviderFetch, withStubbedProviderFetch } from "../helpers/catalog-provider-fetch"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; -import { removeTreeAfterBackgroundWork } from "../helpers/remove-tree"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; +import * as removal from "../helpers/remove-tree"; const GLM = "zai-glm-5-3"; const NATIVE = "mistral-large-4-0"; const IMAGE = "data:image/png;base64,aGVsbG8taW1hZ2UtYnl0ZXM="; const CAPTION = "A red square with a white circle."; -const originalFetch = globalThis.fetch; +let forbiddenProviderCalls = 0; +const nativeFetch = globalThis.fetch; +const originalFetch = ((input, init) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.hostname === "127.0.0.1") return nativeFetch(input, init); + forbiddenProviderCalls++; + throw new Error("Network forbidden outside an active Mistral vision fixture"); +}) as typeof fetch; +const realOutboundGet = outbound.providerOutboundGet; +const realOutboundPost = outbound.providerOutboundPost; +let outboundGet: ReturnType>; +let outboundPost: ReturnType>; let previousHome: string | undefined; let home = ""; let codexHome: IsolatedCodexHome; let sidecarCalls: Array>; let mainCalls: Array>; let proxy: ReturnType | undefined; +let stopServer: () => Promise | undefined; let requestFlight: Promise<{ status: number; body: string }> | undefined; +let requestPending = false; +const ownedCleanup = new Set>(); let cleanupFlight: Promise | undefined; let sidecarWait: Promise | undefined; let sidecarEntered: (() => void) | undefined; +let isolationActive = false; +let cleanupPending = false; +let cleanupRetryUsed = false; +let cleanupFailures: unknown[] = []; + +function assertFixtureReleased(): void { + if (isolationActive || requestPending || ownedCleanup.size) throw new Error("Previous fixture is undrained; refusing to replace its isolation"); +} + +beforeAll(() => { + globalThis.fetch = originalFetch; + // File reload drops function-valued provider.fetch. Keep discovery on its real + // transport path with an injected executor, including late startup work. + outboundGet = spyOn(outbound, "providerOutboundGet").mockImplementation((name, provider, url, init, deps) => + realOutboundGet(name, { ...provider, fetch: stubbedProviderFetch }, url, init, deps)); + outboundPost = spyOn(outbound, "providerOutboundPost").mockImplementation((name, provider, url, init, deps) => + realOutboundPost(name, { ...provider, fetch: stubbedProviderFetch }, url, init, deps)); +}); + +afterAll(async () => { + assertFixtureReleased(); + if (!(await settleBackgroundWork({ timeoutMs: 15_000 })).settled) throw new Error("Suite still owns background work; mocks retained"); + expect(forbiddenProviderCalls).toBe(0); + outboundGet.mockRestore(); + outboundPost.mockRestore(); + globalThis.fetch = nativeFetch; +}); beforeEach(() => { + assertFixtureReleased(); + isolationActive = true; + cleanupPending = cleanupRetryUsed = false; + cleanupFailures = []; previousHome = process.env.OPENCODEX_HOME; codexHome = installIsolatedCodexHome("ocx-mistral-vision-codex-"); home = mkdtempSync(join(tmpdir(), "ocx-mistral-vision-")); @@ -42,7 +90,9 @@ beforeEach(() => { sidecarCalls = []; mainCalls = []; proxy = undefined; + stopServer = () => proxy?.stop(false); requestFlight = cleanupFlight = undefined; + requestPending = false; sidecarWait = undefined; sidecarEntered = undefined; globalThis.fetch = (async (input, init) => { @@ -74,26 +124,73 @@ beforeEach(() => { }) as typeof fetch; }); -function finishFixture(): Promise { +function finishFixture(drainTimeoutMs = 15_000, cleanupTimeoutMs = 10_000): Promise { return cleanupFlight ??= (async () => { - // Background settlement alone does not own HTTP requests. Close ingress gracefully - // and await the complete response body before any fetch mock or home is restored. - await proxy?.stop(false); - await requestFlight?.catch(() => undefined); - await removeTreeAfterBackgroundWork(home); - clearModelCache(); - resetVisionDescriptionCache(); - if (previousHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = previousHome; - codexHome.restore(); - globalThis.fetch = originalFetch; + cleanupPending = true; + try { + // Background settlement alone does not own HTTP requests. Close ingress gracefully + // and await the complete response body before any fetch mock or home is restored. + try { await awaitOwnedCleanup(Promise.resolve().then(stopServer), "server shutdown", cleanupTimeoutMs); } + catch (error) { + cleanupFailures.push(error); + if (ownedCleanup.size) throw new AggregateError(cleanupFailures, "Fixture shutdown is pending; mocks retained"); + // A shutdown hook can fail after closing ingress. Retry gracefully: forcing + // a socket closed could reject the client while its handler still uses fetch. + try { await awaitOwnedCleanup(Promise.resolve().then(stopServer), "server shutdown retry", cleanupTimeoutMs); } + catch (retryError) { + cleanupFailures.push(retryError); + throw new AggregateError(cleanupFailures, "Fixture ingress did not confirm shutdown; mocks retained"); + } + } + try { + if (requestFlight) await awaitOwnedCleanup(requestFlight.catch(() => undefined), "response drain", cleanupTimeoutMs); + } catch (error) { + cleanupFailures.push(error); + throw new AggregateError(cleanupFailures, "Fixture response is pending; mocks retained"); + } + // A drain timeout is not a deletion failure: producers still own fetch and both homes. + try { + const drained = await settleBackgroundWork({ timeoutMs: drainTimeoutMs }); + if (!drained.settled) throw new Error(`Fixture background drain timed out: ${drained.pending.join(", ")}`); + await awaitOwnedCleanup(flushWindowsSecretAclReapsBeforeRemoval(home), "ACL reap", cleanupTimeoutMs); + if (pendingBackgroundWork().length) throw new Error("Fixture background work restarted during reap"); + } catch (error) { + cleanupFailures.push(error); + throw new AggregateError(cleanupFailures, "Fixture background work is undrained; mocks retained"); + } + // No asynchronous producer can start between the final check and guarded removal. + try { removeTreeWithRetry(home); } + catch (error) { cleanupFailures.push(error); } + finally { + clearModelCache(); + resetVisionDescriptionCache(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + try { codexHome.restore(); } + finally { globalThis.fetch = originalFetch; isolationActive = false; } + } + if (cleanupFailures.length) throw new AggregateError(cleanupFailures, "Fixture cleanup failed", { cause: cleanupFailures[0] }); + } finally { cleanupPending = false; } })(); } -afterEach(finishFixture, 60_000); +/** One explicit recovery after the owner releases retained work; preserve the initial failure. */ +function retryFixtureCleanup(): Promise { + if (!isolationActive || cleanupPending || cleanupRetryUsed || requestPending || ownedCleanup.size || pendingBackgroundWork().length) throw new Error("Fixture cleanup recovery is unavailable"); + cleanupRetryUsed = true; + cleanupFlight = undefined; + return finishFixture(); +} + +afterEach(() => finishFixture(), 60_000); + +function ownFixtureRequest(work: Promise<{ status: number; body: string }>) { + requestPending = true; + return requestFlight = work.finally(() => { requestPending = false; }); +} function imageRequest(model: string): Promise<{ status: number; body: string }> { - return requestFlight = (async () => { + return ownFixtureRequest((async () => { const token = fakeChatGptJwt({ chatgpt_account_id: "acct-mistral-vision-fixture" }); const response = await fetch(new URL("/v1/responses", proxy!.url), { method: "POST", @@ -103,7 +200,22 @@ function imageRequest(model: string): Promise<{ status: number; body: string }> ] }] }), }); return { status: response.status, body: await response.text() }; - })(); + })()); +} + +function awaitOwnedCleanup(work: Promise, label: string, timeoutMs = 10_000): Promise { + ownedCleanup.add(work); + void work.then(() => ownedCleanup.delete(work), () => ownedCleanup.delete(work)); + return fixtureDeadline(work, label, timeoutMs); +} + +async function fixtureDeadline(work: Promise, label: string, timeoutMs = 10_000): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([work, new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`Fixture timed out waiting for ${label}`)), timeoutMs); + })]); + } finally { clearTimeout(timer); } } /** Build an isolated Mistral configuration with a caller-authenticated OpenAI vision helper. */ @@ -121,6 +233,16 @@ function configuration(): OcxConfig { } describe("Mistral GLM vision sidecar defaults", () => { + test("file-reloaded provider discovery uses the fixture executor without native outbound transport", async () => { + saveConfig(configuration()); + const reloaded = loadConfig(); + expect(reloaded.providers.mistral?.fetch).toBeUndefined(); + const before = outboundGet.mock.calls.length; + const models = await gatherRoutedModels(reloaded); + expect(outboundGet.mock.calls.length).toBeGreaterThan(before); + expect(buildCatalogEntries(null, [], models).find(row => row.slug === `mistral/${GLM}`)?.input_modalities) + .toEqual(["text", "image"]); + }); test("live discovery publishes GLM attachments without a saved modality override", async () => { const config = configuration(); const models = await gatherRoutedModels(withStubbedProviderFetch(config)); @@ -206,7 +328,7 @@ describe("Mistral GLM vision sidecar defaults", () => { let requestFinished = false; const response = imageRequest(`mistral/${GLM}`).then(value => { requestFinished = true; return value; }); try { - await entered; + await fixtureDeadline(entered, "helper admission"); expect((await settleBackgroundWork({ timeoutMs: 5_000 })).settled).toBe(true); expect(requestFinished).toBe(false); let cleanupFinished = false; @@ -222,4 +344,138 @@ describe("Mistral GLM vision sidecar defaults", () => { expect(sidecarCalls).toHaveLength(1); } finally { release(); } }, 60_000); + + test("fixture restores globals after a drained home removal failure", async () => { + const remove = spyOn(removal, "removeTreeWithRetry").mockImplementationOnce(() => { throw new Error("fixture removal failed"); }); + const attempt = finishFixture(); + cleanupFlight = attempt.catch(() => undefined); + try { + await expect(attempt).rejects.toThrow("Fixture cleanup failed"); + expect(globalThis.fetch).toBe(originalFetch); + expect(process.env.OPENCODEX_HOME).toBe(previousHome); + expect(process.env.CODEX_HOME).not.toBe(codexHome.path); + } finally { + remove.mockRestore(); + removeTreeWithRetry(home); + } + }, 60_000); + + test.each([1, 2])("fixture retains isolation through %s injected shutdown failures", async failures => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const response = ownFixtureRequest(gate.then(() => ({ status: 200, body: "fixture response drained" }))); + let calls = 0; + stopServer = async () => { + if (++calls <= failures) throw new Error("fixture shutdown failed"); + }; + const guardedFetch = globalThis.fetch; + try { + const attempt = finishFixture(); + cleanupFlight = attempt.catch(() => undefined); + let outcome: unknown; + if (failures === 2) { + // Ingress did not close: reject promptly without waiting for the held request. + outcome = await fixtureDeadline(attempt.catch(error => error), "shutdown refusal"); + expect(outcome).toBeInstanceOf(AggregateError); + expect(requestPending).toBe(true); + expect(() => retryFixtureCleanup()).toThrow("recovery is unavailable"); + } else { + for (let tick = 0; tick < 8; tick++) await Promise.resolve(); + expect(cleanupPending).toBe(true); + } + expect(globalThis.fetch).toBe(guardedFetch); + expect(process.env.OPENCODEX_HOME).toBe(home); + release(); + await response; + outcome ??= await fixtureDeadline(attempt.catch(error => error), "cleanup result"); + expect((outcome as AggregateError).errors[0].message).toBe("fixture shutdown failed"); + expect(globalThis.fetch).toBe(failures === 1 ? originalFetch : guardedFetch); + expect(calls).toBe(2); + } finally { + release(); + await response; + stopServer = () => undefined; + if (failures === 2 && isolationActive && !cleanupPending) { + const recovery = retryFixtureCleanup(); + cleanupFlight = recovery.catch(() => undefined); + await expect(recovery).rejects.toThrow("Fixture cleanup failed"); + expect(globalThis.fetch).toBe(originalFetch); + } + } + }, 60_000); + + test("fixture retains isolation after a real background drain timeout until its one recovery", async () => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const guardedFetch = globalThis.fetch; + const observed: { fetch?: typeof fetch; home?: string; codex?: string } = {}; + const work = trackBackgroundWork("held-fixture-observer", gate.then(() => { + observed.fetch = globalThis.fetch; + observed.home = process.env.OPENCODEX_HOME; + observed.codex = process.env.CODEX_HOME; + })); + try { + const attempt = finishFixture(1); + cleanupFlight = attempt.catch(() => undefined); + await expect(attempt).rejects.toThrow("undrained; mocks retained"); + expect(() => assertFixtureReleased()).toThrow("refusing to replace its isolation"); + expect(() => retryFixtureCleanup()).toThrow("recovery is unavailable"); + expect(finishFixture()).toBe(cleanupFlight); + expect(globalThis.fetch).toBe(guardedFetch); + release(); + await work; + expect(observed).toEqual({ fetch: guardedFetch, home, codex: codexHome.path }); + const recovery = retryFixtureCleanup(); + cleanupFlight = recovery.catch(() => undefined); + const failure = await recovery.catch(error => error as AggregateError); + expect(failure).toBeInstanceOf(AggregateError); + expect((failure as AggregateError).errors[0].message).toContain("held-fixture-observer"); + expect(globalThis.fetch).toBe(originalFetch); + expect(process.env.OPENCODEX_HOME).toBe(previousHome); + expect(process.env.CODEX_HOME).not.toBe(codexHome.path); + expect(() => assertFixtureReleased()).not.toThrow(); + expect(() => retryFixtureCleanup()).toThrow("recovery is unavailable"); + } finally { + release(); + await work; + if (isolationActive && !cleanupRetryUsed) { + cleanupFlight = retryFixtureCleanup().catch(() => undefined); + await cleanupFlight; + } + } + }, 60_000); + + test.each(["stop", "request"] as const)("fixture rejects a %s deadline without restoring or retrying live work", async phase => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const guardedFetch = globalThis.fetch; + if (phase === "stop") stopServer = () => gate; + else ownFixtureRequest(gate.then(() => ({ status: 200, body: "released" }))); + try { + const attempt = finishFixture(15_000, 1); + cleanupFlight = attempt.catch(() => undefined); + await expect(attempt).rejects.toThrow("mocks retained"); + expect(globalThis.fetch).toBe(guardedFetch); + expect(process.env.OPENCODEX_HOME).toBe(home); + expect(() => retryFixtureCleanup()).toThrow("recovery is unavailable"); + release(); + await (requestFlight ?? gate); + await fixtureDeadline(Promise.all([...ownedCleanup]), "released cleanup work"); + stopServer = () => undefined; + const recovery = retryFixtureCleanup(); + cleanupFlight = recovery.catch(() => undefined); + const failure = await recovery.catch(error => error as AggregateError); + expect((failure as AggregateError).errors[0].message).toContain("Fixture timed out waiting for"); + expect(globalThis.fetch).toBe(originalFetch); + } finally { + release(); + await (requestFlight ?? gate); + await fixtureDeadline(Promise.all([...ownedCleanup]), "released cleanup work"); + stopServer = () => undefined; + if (isolationActive && !cleanupRetryUsed && !cleanupPending) { + cleanupFlight = retryFixtureCleanup().catch(() => undefined); + await cleanupFlight; + } + } + }, 60_000); }); From 18560b74c43884bbe51d76348e1327fe81e1b55d Mon Sep 17 00:00:00 2001 From: Epinephrine <27862058+luvs01@users.noreply.github.com> Date: Tue, 20 Oct 2026 12:58:22 +0900 Subject: [PATCH 3/7] fix(gui): confirm only sidecar edits included in a save --- gui/src/pages/claude-code-save.ts | 23 +++++++++++--- gui/src/pages/claude-code-sidecar.ts | 4 +-- gui/tests/claude-code-save.test.ts | 47 ++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/gui/src/pages/claude-code-save.ts b/gui/src/pages/claude-code-save.ts index 8196de3196e..f494c14fbc0 100644 --- a/gui/src/pages/claude-code-save.ts +++ b/gui/src/pages/claude-code-save.ts @@ -139,8 +139,23 @@ export function acknowledgeSave(current: ClaudeCodeEditState | null, submitted: const baseline = savedCopy(current.baseline.state, submitted); const rows = baseline.rows; const unchanged = claudeCodeDraftKey(current.draft) === claudeCodeDraftKey(submitted); - // Untouched since submission: show what was stored. Edited meanwhile: keep the edits, dirty. - return { draft: unchanged ? { state: { ...current.draft.state, - visionSidecar: baseline.state.visionSidecar, webSearchSidecar: baseline.state.webSearchSidecar, - }, rows } : current.draft, baseline, adoptNextRead: unchanged }; + if (unchanged) { + // Untouched since submission: show what was stored. + return { draft: { state: { ...current.draft.state, + visionSidecar: baseline.state.visionSidecar, webSearchSidecar: baseline.state.webSearchSidecar, + }, rows }, baseline, adoptNextRead: true }; + } + /* + * Edited while the PUT was out: keep the edits, dirty. A sidecar is still confirmed — + * ending its draft-only activation provenance — when its serialized shape is exactly what + * the Save stored; one edited after submission keeps its marker until its own save lands. + */ + let draft = current.draft; + for (const key of ["visionSidecar", "webSearchSidecar"] as const) { + const stored = JSON.stringify(serializeSidecarOverride(submitted.state[key])); + if (JSON.stringify(serializeSidecarOverride(draft.state[key])) !== stored) continue; + const confirmed = confirmSidecarOverride(draft.state[key]); + if (confirmed !== draft.state[key]) draft = { ...draft, state: { ...draft.state, [key]: confirmed } }; + } + return { draft, baseline, adoptNextRead: false }; } diff --git a/gui/src/pages/claude-code-sidecar.ts b/gui/src/pages/claude-code-sidecar.ts index 36407ed8cac..6480a48db4a 100644 --- a/gui/src/pages/claude-code-sidecar.ts +++ b/gui/src/pages/claude-code-sidecar.ts @@ -12,9 +12,9 @@ export type SidecarSelectValue = "inherit" | "auto" | VisionOverrideBackend; const modelEditStartedDisabled = Symbol("modelEditStartedDisabled"); type SidecarDraft = SidecarOverride & { [modelEditStartedDisabled]?: true }; -/** A confirmed save starts a new editing baseline. */ +/** A confirmed save starts a new editing baseline; drafts without a pending marker keep their identity. */ export function confirmSidecarOverride(override?: SidecarOverride): SidecarOverride | undefined { - if (!override) return undefined; + if (!override || !(override as SidecarDraft)[modelEditStartedDisabled]) return override; const next = { ...override } as SidecarDraft; delete next[modelEditStartedDisabled]; return next; diff --git a/gui/tests/claude-code-save.test.ts b/gui/tests/claude-code-save.test.ts index b40ac3e058d..9d95622d821 100644 --- a/gui/tests/claude-code-save.test.ts +++ b/gui/tests/claude-code-save.test.ts @@ -11,6 +11,7 @@ import { } from "../src/pages/claude-code-save"; import { normalizeContextAccounting, type ClaudeCodeState, type MapRow } from "../src/pages/claude-code-types"; import { applySidecarBackendChange, applySidecarModelChange, applySidecarPoolChange } from "../src/pages/claude-code-sidecar"; +import type { SidecarOverride } from "../src/pages/claude-manual-env"; const STATE = { enabled: true, @@ -169,3 +170,49 @@ test("context accounting is saved, dirties the draft, and older states read as t expect(normalizeContextAccounting("bogus")).toBe("1m"); expect(normalizeContextAccounting("200k")).toBe("200k"); }); + +const sidecarTyped = (disabled: SidecarOverride | undefined, model: string) => + applySidecarBackendChange(applySidecarModelChange(disabled, model), "openai"); + +test("a stored sidecar is confirmed even when an unrelated field was edited while the PUT was out", () => { + const base = editable({ visionSidecar: { enabled: false }, webSearchSidecar: { enabled: false } }); + const vision = sidecarTyped(base.state.visionSidecar, "gpt-5.6-luna"); + const web = sidecarTyped(base.state.webSearchSidecar, "claude-haiku-4-5"); + const submitted = editable({ visionSidecar: vision, webSearchSidecar: web }); + // Only an unrelated field changed mid-flight; the sidecars the server stored are unchanged. + const pending = editable({ visionSidecar: vision, webSearchSidecar: web, authMode: "proxy" }); + const acked = acknowledgeSave({ draft: pending, baseline: base, adoptNextRead: false, savePending: true }, submitted)!; + expect(acked.adoptNextRead).toBe(false); + expect(acked.draft.state.authMode).toBe("proxy"); + // Confirmed enabled=true: clearing the model afterwards keeps the saved activation. + expect(applySidecarModelChange(acked.draft.state.visionSidecar, "").enabled).toBe(true); + expect(applySidecarModelChange(acked.draft.state.webSearchSidecar, "").enabled).toBe(true); +}); + +test("a sidecar edited after submission keeps its disabled-start marker until its own save", () => { + const base = editable({ visionSidecar: { enabled: false } }); + const typed = sidecarTyped(base.state.visionSidecar, "gpt-5.6-luna"); + const submitted = editable({ visionSidecar: typed }); + // Cleared mid-flight: the serialized shape no longer matches what the PUT stored. + const cleared = applySidecarModelChange(typed, ""); + const acked = acknowledgeSave({ draft: editable({ visionSidecar: cleared }), baseline: base, adoptNextRead: false }, submitted)!; + // The post-submit edit itself is preserved, not overwritten by the confirmed copy. + expect(acked.draft.state.visionSidecar).toBe(cleared); + expect(isClaudeCodeDraftDirty(acked.draft, acked.baseline)).toBe(true); + // Retyping and clearing again still returns to disabled: this draft never reached the server. + expect(applySidecarModelChange(applySidecarModelChange(acked.draft.state.visionSidecar, "gpt-5.6-luna"), "").enabled).toBe(false); +}); + +test("stored and post-submit-edited sidecars are confirmed independently", () => { + const base = editable({ visionSidecar: { enabled: false }, webSearchSidecar: { enabled: false } }); + const vision = sidecarTyped(base.state.visionSidecar, "gpt-5.6-luna"); + const web = sidecarTyped(base.state.webSearchSidecar, "claude-haiku-4-5"); + const submitted = editable({ visionSidecar: vision, webSearchSidecar: web }); + // Vision's model changed mid-flight (still enabled), webSearch did not: each is decided on its own. + const visionRetyped = applySidecarModelChange(vision, "gpt-5.6-astra"); + const acked = acknowledgeSave({ draft: editable({ visionSidecar: visionRetyped, webSearchSidecar: web }), baseline: base, adoptNextRead: false }, submitted)!; + expect(applySidecarModelChange(acked.draft.state.webSearchSidecar, "").enabled).toBe(true); + // Vision stays enabled in the draft, so only its retained marker turns a clear back into disabled. + expect(acked.draft.state.visionSidecar).toBe(visionRetyped); + expect(applySidecarModelChange(acked.draft.state.visionSidecar, "").enabled).toBe(false); +}); From 9e5a917eddb8d1f1ab32dc0421e415ddb71ff3e9 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:25:44 +0900 Subject: [PATCH 4/7] fix(test): promote Bun 1.4.4 with independent runner role verification --- .github/actions/setup-project-bun/action.yml | 4 +-- .github/workflows/ci.yml | 10 +++--- CONTRIBUTING.md | 6 ++-- docs-site/src/content/docs/contributing.md | 6 ++-- package.json | 2 +- structure/runtime.md | 4 +-- tests/ci-workflows/ci-scope-gaps.test.ts | 2 +- tests/ci-workflows/install-scripts.test.ts | 6 ++-- tests/test-layout-tooling.test.ts | 34 ++++++++++++++++---- 9 files changed, 48 insertions(+), 26 deletions(-) diff --git a/.github/actions/setup-project-bun/action.yml b/.github/actions/setup-project-bun/action.yml index 1ab22dd1dc9..cf28f6f9d43 100644 --- a/.github/actions/setup-project-bun/action.yml +++ b/.github/actions/setup-project-bun/action.yml @@ -8,8 +8,8 @@ description: >- # (#6713 run 37631558263: Linux 1/4, macOS 2/2, Windows 9/9). It moved from 1.4.0 to # 1.4.3 for oven-sh/bun#44581: 1.4.0's Bun.spawnSync could lose a child's exit after # finalizers ran inside an earlier call and spin past its own timeout (#4956). -# Rejoin only after the upstream runner crash is fixed and a green lane=all -# dispatch proves the unified version; then delete testRunnerBun and this role input. +# Both roles now default to the package's 1.4.4 security transition, retaining role validation. +# Promotion requires a green full cross-platform lane=all run on the exact candidate head. inputs: role: description: Bun role to install (runtime or test-runner). diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 028c28ca318..7bcd0eaa3f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -578,12 +578,10 @@ jobs: printf 'Scheduled run: workflow from %s, testing dev at %s\n' "$GITHUB_SHA" "$sha" >> "$GITHUB_STEP_SUMMARY" printf 'scheduled run tests dev at %s (workflow %s)\n' "$sha" "$GITHUB_SHA" - # Bun roles (#6713): runtime 1.4.2 fixes #6684's Windows Bun.fetch streaming; - # the test runner skipped 1.4.2's multi-file --isolate preload segfault (#4821 - # reverted #4064; run 37631558263: Linux 1/4, macOS 2/2, Windows 9/9) and runs - # 1.4.3, whose oven-sh/bun#44581 stops Bun.spawnSync losing a child's exit (#4956). - # Rejoin after the upstream runner crash is fixed and a green lane=all dispatch - # proves the unified version, then delete testRunnerBun and the action's role input. + # Bun runtime and test-runner roles both read their 1.4.4 defaults from package.json. + # Keep role validation after the historical isolate preload and spawnSync failures + # (#6713, #4956). Promotion requires a green full cross-platform lane=all run on + # the exact candidate head; focused local results do not replace that proof. # The suite, split by file across four Linux runners. # # `scripts/ci/run-bun-test-batches.sh` assigns files to shards by the per-file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7d4aa94f580..bbcd248f8f8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,9 +69,11 @@ A ready-for-review PR is the author's claim that the change is complete, underst ## Local validation and hooks -The suite uses Bun 1.4.3 (`testRunnerBun`) while the shipped runtime is 1.4.4. -`scripts/test.ts` finds a 1.4.3 binary on PATH or in `~/.bun/bin`, or uses +The suite uses Bun 1.4.4 (`testRunnerBun`) while the shipped runtime is 1.4.4. +`scripts/test.ts` finds a 1.4.4 binary on PATH or in `~/.bun/bin`, or uses `OCX_TEST_RUNNER_BUN`; install that version locally if it is missing. +The roles remain independently checked even when their versions match. Promotion requires +a successful full cross-platform `lane=all` run on the candidate head. Run `bun run test` before review readiness. If the full local suite is too costly for the task or available resources, run at least focused regression tests for diff --git a/docs-site/src/content/docs/contributing.md b/docs-site/src/content/docs/contributing.md index 7fe0b09831b..d0021b7f12a 100644 --- a/docs-site/src/content/docs/contributing.md +++ b/docs-site/src/content/docs/contributing.md @@ -45,9 +45,11 @@ bun run privacy:scan # credential/privacy scan used by CI bun run prepare:package # refresh package launchers/assets ``` -The suite uses Bun 1.4.3 (`package.json`'s `testRunnerBun`) while the shipped runtime is 1.4.4. -`scripts/test.ts` finds a 1.4.3 binary on PATH or in `~/.bun/bin`, or uses `OCX_TEST_RUNNER_BUN`. +The suite uses Bun 1.4.4 (`package.json`'s `testRunnerBun`) while the shipped runtime is 1.4.4. +`scripts/test.ts` finds a 1.4.4 binary on PATH or in `~/.bun/bin`, or uses `OCX_TEST_RUNNER_BUN`. Install that version locally if it is missing; the resolver never downloads it. +The roles remain independently checked even when their versions match. Promotion requires +a successful full cross-platform `lane=all` run on the candidate head. Run `bun run test` by default. If a full run is disproportionately expensive for the task size, available machine resources, or concurrent worktrees, you must at least run focused regression tests diff --git a/package.json b/package.json index 89176da2851..a270427faf2 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "trustedDependencies": [ "bun" ], - "testRunnerBun": "1.4.3", + "testRunnerBun": "1.4.4", "scripts": { "dev": "bun run src/cli/index.ts start", "dev:proxy": "bun run src/cli/index.ts start", diff --git a/structure/runtime.md b/structure/runtime.md index 43e071d457f..763f3e90c3d 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -4,7 +4,7 @@ The minute sweep checks persisted activation deadlines locally; only missing dea ## Bun runtime and test runner -`package.json` pins the shipped npm runtime (`dependencies.bun`) to 1.4.4 and the test runner (`testRunnerBun`) to 1.4.3; `@types/bun` remains unchanged. Runtime 1.4.2 fixed #6684's Windows Bun.fetch truncation after the first cleartext upstream body chunk; 1.4.4 keeps that fix and ships NSS 3.131.1 roots, which drop the compromised eMudhra CAs. +`package.json` pins both the shipped npm runtime (`dependencies.bun`) and the test runner (`testRunnerBun`) to 1.4.4; the roles remain independently validated and `@types/bun` remains unchanged. Runtime 1.4.2 fixed #6684's Windows Bun.fetch truncation after the first cleartext upstream body chunk; 1.4.4 keeps that fix and ships NSS 3.131.1 roots, which drop the compromised eMudhra CAs. The runner pin skipped 1.4.2's multi-file `bun test --isolate` preload reload segfault (`load_preloads -> JSModuleLoader::loadModule -> JSPromise::status`, address 0x10): #4064 introduced 1.4.2 on 2026-09-09 and #4821 reverted it on 2026-09-17. It left 1.4.0 for 1.4.3 because of oven-sh/bun#44581 (#4956). Bun 1.4.0's `Bun.spawnSync` pointed the VM at its private wait loop, so a finalizer that ran during one call (an earlier file's stdout/stderr sink, a stream reader) released its poll on that loop; the cached loop's poll count drifted to zero and a later call stopped polling for its child, spinning past its own `timeout` until the 120-second batch deadline. In a multi-file `--isolate` process that depends on what earlier files left to finalize, so single-file attribution stayed clean. `tests/ci-workflows/test-runner-spawnsync-finalizer.test.ts` reproduces the mechanism in a child under a bounded parent and fails on 1.4.0. #6713 run 37631558263 reproduces that 1.4.2 preload segfault on Linux 1/4, macOS 2/2 and Windows 9/9; single-file success does not prove the multi-file runner works. @@ -12,7 +12,7 @@ The runner pin skipped 1.4.2's multi-file `bun test --isolate` preload reload se Test jobs, including single-file development-version validation, select `test-runner`. They also build dashboard fixtures; `gates` uploads a dashboard preview and checks release-helper syntax on that pin. Release packaging, Docker, installed-artifact and service smoke jobs, desktop shell and widget preparation use `runtime`. Shipped binaries compile through `process.execPath` in `scripts/build-standalone.ts` and `desktop/scripts/prepare-sidecar.ts` on the runtime pin. -`scripts/lib/test-runner-bun.ts` selects the test pin for `scripts/test.ts`, `scripts/release.ts` preflight, `scripts/test-layout/verify.ts`, provider-option gates and GUI package tests via `scripts/test-with-pinned-bun.ts`: matching current Bun first, then a version-checked `OCX_TEST_RUNNER_BUN`, then PATH outside node_modules and `~/.bun/bin`. Missing or mismatched binaries fail with install guidance, without downloading; package-script PATH shadowing cannot select the shipped runtime for tests. Rejoin only after an upstream fix and green unified `lane=all` proof; then remove the runner pin, resolver and CI role split. +`scripts/lib/test-runner-bun.ts` selects the test pin for `scripts/test.ts`, `scripts/release.ts` preflight, `scripts/test-layout/verify.ts`, provider-option gates and GUI package tests via `scripts/test-with-pinned-bun.ts`: matching current Bun first, then a version-checked `OCX_TEST_RUNNER_BUN`, then PATH outside node_modules and `~/.bun/bin`. Missing or mismatched binaries fail with install guidance, without downloading. The caller need not already run the test pin; only a version match permits reusing its executable. `tests/test-layout-tooling.test.ts` verifies matching/different caller fixtures, the selected isolate command, and refusal before spawning tests for a wrong-version override. The 1.4.4 security transition aligns the default versions while retaining the role fields, resolver and CI role checks. Promotion requires successful full cross-platform `lane=all` proof on the exact candidate head; focused local tests do not establish that proof. ## Resolved static model policy diff --git a/tests/ci-workflows/ci-scope-gaps.test.ts b/tests/ci-workflows/ci-scope-gaps.test.ts index f780d71aa90..0729ab6945d 100644 --- a/tests/ci-workflows/ci-scope-gaps.test.ts +++ b/tests/ci-workflows/ci-scope-gaps.test.ts @@ -65,7 +65,7 @@ describe("an edit to the setup action alone", () => { }); }); -// #6713: user runtimes and compiled artifacts need 1.4.2; isolate runners need the testRunnerBun pin (1.4.3, #4956). +// Keep runtime and test-runner role selection distinct even when package.json pins both to 1.4.4. // Check the active setup at each test command, so a later role switch cannot evade the guard. const runsBunTests = (step: Step): boolean => /\bbun\s+test\b|\bbun\s+run\s+test(?::changed)?(?:\s|$)|\bbun\s+(?:run\s+)?scripts\/test\.ts\b|scripts\/ci\/run-bun-test-batches\.sh/.test( diff --git a/tests/ci-workflows/install-scripts.test.ts b/tests/ci-workflows/install-scripts.test.ts index 51e62b0b6fc..23ed7b7461e 100644 --- a/tests/ci-workflows/install-scripts.test.ts +++ b/tests/ci-workflows/install-scripts.test.ts @@ -94,10 +94,10 @@ describe("install scripts", () => { // and spun past its own timeout. Scratch run 38213764818 stalled 8 of 252 Linux shard 1/4 // batch-5 runs that way (CLI child a zombie, its pidfd armed but never polled). // tests/ci-workflows/test-runner-spawnsync-finalizer.test.ts keeps the runner above it. - // Rejoin after the upstream runner crash is fixed and a green `lane=all` dispatch - // proves the unified version; then delete testRunnerBun and the setup action's role. + // The security transition now defaults both roles to 1.4.4. Keep their independent + // validation; promotion still requires a green full cross-platform lane=all candidate run. expect(pkg.dependencies?.bun).toBe("1.4.4"); - expect(pkg.testRunnerBun).toBe("1.4.3"); + expect(pkg.testRunnerBun).toBe("1.4.4"); expect(pkg.dependencies?.zod).toBe("4.4.3"); expect(pkg.devDependencies?.typescript).toBe("7.0.2"); expect(pkg.devDependencies?.["@types/bun"]).toBe("1.4.0"); diff --git a/tests/test-layout-tooling.test.ts b/tests/test-layout-tooling.test.ts index 354e3943fe0..cb4a7482819 100644 --- a/tests/test-layout-tooling.test.ts +++ b/tests/test-layout-tooling.test.ts @@ -7,6 +7,7 @@ import { repoPath, repoRoot } from "./helpers/repo-root"; import { listTestFiles, planMoves } from "../scripts/test-layout/plan"; import { runMove } from "../scripts/test-layout/move"; import { runVerify } from "../scripts/test-layout/verify"; +import * as runnerBun from "../scripts/lib/test-runner-bun"; import { testRunnerBun } from "../package.json"; import { anchors, @@ -469,24 +470,43 @@ describe("move end to end", () => { } }); - test("domain verification launches isolate tests through the pinned runner", () => { + const otherBunVersion = testRunnerBun === "1.4.3" ? "1.4.4" : "1.4.3"; + test.each([ + ["matching caller", testRunnerBun, testRunnerBun, false], + ["different caller", otherBunVersion, testRunnerBun, false], + ["wrong override version", otherBunVersion, otherBunVersion, true], + ] as const)("domain verification preserves the configured runner role: %s", (_name, currentVersion, overrideVersion, refused) => { const { root, cleanup } = scratchRepo(); const layoutPath = join(root, "scripts", "test-layout", "layout.json"); try { runMove({ root, domains: ["server", "providers"], dryRun: false, layoutPath, skipVerify: true, log: () => {} }); + const binaryName = process.platform === "win32" ? "bun.exe" : "bun"; + const pinnedRunner = join(root, "test-runner", binaryName); + const caller = currentVersion === testRunnerBun ? pinnedRunner : join(root, "runtime", binaryName); + const probes: string[] = []; + // Keep role resolution real, but version probes independent of the host and the spawn mock. + const select = spyOn(runnerBun, "getTestRunnerBun").mockImplementation(() => runnerBun.resolveTestRunnerBun({ + pin: testRunnerBun, currentVersion, execPath: caller, + env: { OCX_TEST_RUNNER_BUN: pinnedRunner }, pathEntries: [], homeDir: root, platform: process.platform, + probeVersion: binary => { probes.push(binary); return binary === pinnedRunner ? overrideVersion : undefined; }, + })); const spawn = spyOn(Bun, "spawnSync").mockReturnValue({ exitCode: 0, stdout: Buffer.from(""), stderr: Buffer.from(""), } as ReturnType); try { - const report = runVerify({ root, domains: ["server", "providers"], layoutPath, log: () => {} }); - expect(report.testExit).toBe(0); - // The test itself runs on the required pin, so its executable is the resolver's first choice. - expect(Bun.version).toBe(testRunnerBun); - expect(spawn.mock.calls.at(-1)?.[0]).toEqual([ - process.execPath, "test", "--isolate", "tests/server", "tests/providers", + const verify = () => runVerify({ root, domains: ["server", "providers"], layoutPath, log: () => {} }); + if (refused) expect(verify).toThrow(`OCX_TEST_RUNNER_BUN reports ${overrideVersion}`); + else expect(verify()).toMatchObject({ ok: true, testExit: 0 }); + expect(select).toHaveBeenCalledTimes(1); + expect(probes).toEqual(currentVersion === testRunnerBun ? [] : [pinnedRunner]); + const testSpawns = spawn.mock.calls.filter(([command]) => Array.isArray(command) && command[1] === "test"); + expect(testSpawns.map(([command]) => command)).toEqual(refused ? [] : [ + [pinnedRunner, "test", "--isolate", "tests/server", "tests/providers"], ]); + if (!refused) expect(testSpawns[0]?.[1]).toMatchObject({ cwd: root, stdout: "inherit", stderr: "inherit" }); } finally { spawn.mockRestore(); + select.mockRestore(); } } finally { cleanup(); From 1deb67d274c8246df9ad539bff93f958782032cb Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 21:38:09 +0900 Subject: [PATCH 5/7] docs: align Docker Bun comments with unified role defaults --- Dockerfile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 82fe86f4cf1..54e0153e0f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,12 +2,12 @@ # Keep the runtime aligned with package.json and pin the multi-platform image index. # Runtime 1.4.4 keeps 1.4.2's #6684 Windows Bun.fetch streaming fix and ships NSS -# 3.131.1 roots without the compromised eMudhra CAs; CI tests run on 1.4.3 -# because 1.4.2 segfaults reloading preloads in multi-file --isolate (#4821; -# #6713 run 37631558263: Linux 1/4, macOS 2/2, Windows 9/9) and 1.4.0's spawnSync -# can lose a child's exit (#4956, oven-sh/bun#44581). Rejoin after the -# upstream crash fix passes a unified-version lane=all dispatch, then delete -# package.json's testRunnerBun pin and setup-project-bun's role input. +# 3.131.1 roots without the compromised eMudhra CAs. Runtime and test-runner +# defaults are both 1.4.4; keep package.json's testRunnerBun pin and +# setup-project-bun's role input independently validated. Promotion requires +# an exact-head cross-platform lane=all dispatch, including multi-file --isolate: +# historical 1.4.2 preload crashes (#4821; #6713 run 37631558263) and 1.4.0 +# spawnSync exit loss (#4956, oven-sh/bun#44581) remain regression risks. ARG BUN_IMAGE=oven/bun:1.4.4@sha256:1d22548f177f4ff38e0c15baa3c2c46c6c2060ba2d1bc573bb4720b407548f1d FROM ${BUN_IMAGE} AS manifest From 41f08fffd3e7dea64a6eed79af918aed74a153dd Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 22:07:35 +0900 Subject: [PATCH 6/7] test: use a draining TLS origin for early upload replies --- .../claude-picker-upload.test.ts | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/tests/claude-integration/claude-picker-upload.test.ts b/tests/claude-integration/claude-picker-upload.test.ts index 8d4cd0d1859..95c6351f04d 100644 --- a/tests/claude-integration/claude-picker-upload.test.ts +++ b/tests/claude-integration/claude-picker-upload.test.ts @@ -3,14 +3,39 @@ import { Agent, createServer, request } from "node:https"; import { connect, constants, type ClientHttp2Session, type ClientHttp2Stream } from "node:http2"; import type { ClientRequest, IncomingMessage, ServerResponse } from "node:http"; import type { Duplex } from "node:stream"; +import { createServer as createTlsServer } from "node:tls"; import { createLocalInterceptCa, issueLocalInterceptLeaf } from "../../src/claude/intercept/local-ca"; import { startPickerListener, type PickerListenerOptions } from "../../src/claude/intercept/picker-listener"; -async function fixture(handler: (req: IncomingMessage, res: ServerResponse) => void, +async function fixture(handler: ((req: IncomingMessage, res: ServerResponse) => void) | "early-fin", options: Partial = {}) { const ca = createLocalInterceptCa(); const leaf = issueLocalInterceptLeaf(ca, ["claude.ai"]); - const upstream = createServer({ cert: leaf.certPem, key: leaf.keyPem }, handler); + const earlyReplies: Array<{ requestComplete: boolean; flushed: boolean }> = []; + // A wire-level origin sends the reply and FIN while continuing to drain input. Bun's HTTP + // server also tears down unfinished requests; overriding destroySoon alone does not prevent + // that reset racing ahead of the reply on Windows with a buffered upload. + const upstream = handler === "early-fin" ? createTlsServer({ cert: leaf.certPem, key: leaf.keyPem, allowHalfOpen: true }, socket => { + socket.on("error", () => socket.destroy()); + let headers = ""; + let replied = false; + socket.on("data", chunk => { + if (replied) return; + headers += chunk.toString("latin1"); + const end = headers.indexOf("\r\n\r\n"); + if (end < 0) return; + replied = true; + const early = headers.startsWith("POST /early "); + const body = early ? "early" : "ordinary"; + const length = /\r\ncontent-length: (\d+)\r\n/i.exec(headers.slice(0, end + 2)); + const requestBody = headers.slice(end + 4); + const observation = { requestComplete: length ? requestBody.length >= Number(length[1]) + : requestBody.endsWith("0\r\n\r\n"), flushed: false }; + if (early) earlyReplies.push(observation); + socket.once("finish", () => { observation.flushed = true; }); + socket.end(`HTTP/1.1 ${early ? "413 Payload Too Large" : "200 OK"}\r\nContent-Length: ${body.length}\r\nConnection: close\r\n\r\n${body}`); + }); + }) : createServer({ cert: leaf.certPem, key: leaf.keyPem }, handler); const sockets = new Set(); for (const event of ["connection", "secureConnection"] as const) upstream.on(event, socket => { sockets.add(socket); socket.once("close", () => sockets.delete(socket)); @@ -34,7 +59,7 @@ async function fixture(handler: (req: IncomingMessage, res: ServerResponse) => v rejectUnauthorized: true, agent: h1Agent, path, method, headers: { Host: "claude.ai", ...headers } }); clients.push(req); return req; }; - return { logs, h2, h1, relay, + return { logs, h2, h1, relay, earlyReplies, async close() { const closed = clients.map(client => client.destroyed ? Promise.resolve() : observeClose(client)); await relay.close(); @@ -353,16 +378,7 @@ for (const protocol of ["h1", "h2"] as const) { // "buffered" answers early while 4 MiB of an advertised 32 MiB body is still queued. for (const input of protocol === "h1" ? ["small", "buffered"] as const : ["small"] as const) bounded(`${protocol} early upstream response closes ${input === "buffered" ? "a buffered" : "an"} unfinished upload without losing the reply`, async wait => { - const f = await fixture((req, res) => { - // Answer like a real origin that replies early: keep reading the body and close with a FIN. - // Bun's own server would close this unfinished request with destroySoon(), a reset that - // Windows delivers before the reply bytes, so the relay could never read what it should relay. - req.on("error", () => res.destroy()); req.resume(); - const socket = req.socket; - socket.destroySoon = () => { socket.end(); return socket; }; - if (req.url === "/early") { res.writeHead(413, { "Content-Length": "5" }); res.end("early"); } - else res.end("ordinary"); - }); + const f = await fixture("early-fin"); try { const session = f.h2(); const upload = protocol === "h2" @@ -373,6 +389,7 @@ for (const protocol of ["h1", "h2"] as const) { upload.write(input === "buffered" ? Buffer.alloc(4 * 1024 * 1024, 0x61) : "x"); expect(await wait(result)).toEqual({ status: 413, text: "early" }); await wait(closed); + expect(f.earlyReplies).toEqual([{ requestComplete: false, flushed: true }]); if (protocol === "h2") expect((upload as ClientHttp2Stream).rstCode).toBe(constants.NGHTTP2_NO_ERROR); expect(await wait(normalH2(session))).toEqual({ status: 200, text: "ordinary" }); } finally { await wait(f.close()); } From a9c3d127d3728f50533df32504b787823b5220ee Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Fri, 23 Oct 2026 22:09:36 +0900 Subject: [PATCH 7/7] test: confine draining origin to buffered HTTP/1 upload --- .../claude-integration/claude-picker-upload.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/claude-integration/claude-picker-upload.test.ts b/tests/claude-integration/claude-picker-upload.test.ts index 95c6351f04d..d0bb998b873 100644 --- a/tests/claude-integration/claude-picker-upload.test.ts +++ b/tests/claude-integration/claude-picker-upload.test.ts @@ -378,7 +378,16 @@ for (const protocol of ["h1", "h2"] as const) { // "buffered" answers early while 4 MiB of an advertised 32 MiB body is still queued. for (const input of protocol === "h1" ? ["small", "buffered"] as const : ["small"] as const) bounded(`${protocol} early upstream response closes ${input === "buffered" ? "a buffered" : "an"} unfinished upload without losing the reply`, async wait => { - const f = await fixture("early-fin"); + const f = await fixture(input === "buffered" ? "early-fin" : (req, res) => { + // Answer like a real origin that replies early: keep reading the body and close with a FIN. + // Bun's own server would close this unfinished request with destroySoon(), a reset that + // Windows delivers before the reply bytes, so the relay could never read what it should relay. + req.on("error", () => res.destroy()); req.resume(); + const socket = req.socket; + socket.destroySoon = () => { socket.end(); return socket; }; + if (req.url === "/early") { res.writeHead(413, { "Content-Length": "5" }); res.end("early"); } + else res.end("ordinary"); + }); try { const session = f.h2(); const upload = protocol === "h2" @@ -389,7 +398,7 @@ for (const protocol of ["h1", "h2"] as const) { upload.write(input === "buffered" ? Buffer.alloc(4 * 1024 * 1024, 0x61) : "x"); expect(await wait(result)).toEqual({ status: 413, text: "early" }); await wait(closed); - expect(f.earlyReplies).toEqual([{ requestComplete: false, flushed: true }]); + if (input === "buffered") expect(f.earlyReplies).toEqual([{ requestComplete: false, flushed: true }]); if (protocol === "h2") expect((upload as ClientHttp2Stream).rstCode).toBe(constants.NGHTTP2_NO_ERROR); expect(await wait(normalH2(session))).toEqual({ status: 200, text: "ordinary" }); } finally { await wait(f.close()); }