Skip to content

feat(models): configure ApplyPatch editing per model - #5533

Open
Sun-GLiang wants to merge 4 commits into
apache:mainfrom
Sun-GLiang:codex/model-apply-patch-settings
Open

Sun-GLiang wants to merge 4 commits into
apache:mainfrom
Sun-GLiang:codex/model-apply-patch-settings

Conversation

@Sun-GLiang

@Sun-GLiang Sun-GLiang commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Model settings could not control ApplyPatch, and the Codex OAuth path filtered it out. Add an Enabled/Disabled setting per connection and model, with defaults from a shared model support list; unlisted models default to disabled and explicit user choices take precedence.

Enabled models receive ApplyPatch instead of Write/Edit: native structured patches where supported, Codex custom freeform patches, or portable function calls for other interfaces including DeepSeek. Preserve tool history across these protocols and when patch editing is disabled.

ApplyPatch defaults by model

ApplyPatch is enabled by default for these 22 model IDs:

  • gpt-5-codex
  • gpt-5.1
  • gpt-5.1-codex
  • gpt-5.1-codex-mini
  • gpt-5.1-codex-max
  • gpt-5.2
  • gpt-5.2-codex
  • gpt-5.3-codex
  • gpt-5.3-codex-spark
  • gpt-5.4
  • gpt-5.4-mini
  • gpt-5.4-nano
  • gpt-5.4-pro
  • gpt-5.5
  • gpt-5.6
  • gpt-5.6-sol
  • gpt-5.6-terra
  • gpt-5.6-luna
  • gpt-6-astra
  • deepseek-v4-flash
  • deepseek-v4-flash-vision-exp
  • deepseek-v4-pro

All other model IDs default to disabled. Dated snapshots ending in -YYYY-MM-DD inherit the default of their base model ID. An explicit Enabled/Disabled choice saved for a connection and model overrides these defaults.

Verification

  • GitHub CI and Windows recovery passed on 6401e1297, including desktop E2E, Storybook smoke, layout checks, and installed CLI release-candidate validation.
  • Removed the unused Codex protocol declaration. Reproduced the CI failure locally: the hosted DeepSeek tool snapshot still expected Write/Edit after ApplyPatch became enabled by default. Updated the frozen tool hash/list and asserted the portable function-call schema.
  • 359 focused core/runtime tests passed. The full Runtime Host suite passed: 1,979 passed, 12 skipped, including the previously failing hosted wire-contract test.
  • Full build, full typecheck, lint, format, and both Knip workspace checks (Desktop and UI) passed in the isolated PR worktree after a fresh lockfile install. This supersedes the earlier shared-dependency build/typecheck limitation.
  • Storybook production build and the ModelCapabilities / ModelParameterSave browser scenarios passed during the initial implementation. Codex streaming/results use simulated provider responses; no live model endpoint test was run. Full workspace tests were not run locally.

Before / after (production settings components rendered in Storybook):

Before After
Before After

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the model setting, provider routing, history conversion, tests, and this PR description. The code commit includes Generated-by: OpenAI Codex.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 20, 2026
Remove the unused Codex ApplyPatch protocol declaration and update the headless DeepSeek wire contract for default-enabled portable patch editing.

Generated-by: OpenAI Codex
The hosted patch contract passes. The unrelated Side Chat cleanup scenario and its five disposal tests pass locally; trigger another CI run because upstream rerun permissions are unavailable.

Generated-by: OpenAI Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core design holds up: single shared APPLY_PATCH_MODELS list (all 22 IDs verified present in the model-metadata snapshot — no typos), override ?? list precedence with strict codec validation, profile frozen per backend generation so mid-turn consistency holds, backend invalidation unconditional on committed mutations, replay normalization covering all six profile-direction pairs, Write/Edit never exposed alongside apply_patch, and plan-mode/agent-catalog gating consistent.

One P1 inline, one P2 and two P3s below.

P2 — the headless-coding-v1 system prompt tells the model to prefer tools that routing just removed (packages/runtime-host/src/server/hosted-execution-tool-profile.ts:40). HEADLESS_CODING_V1_TOOL_NAMES includes Write, Edit and apply_patch, so routing strips Write/Edit on enabled models — but the prompt still says "Prefer Read, Glob, and Grep for inspection, Edit and Write for file changes" and never names apply_patch. Every such session opens pointing at absent tools → wasted invalid calls until the model discovers apply_patch from the schema list. Pre-existing for openai-structured on main; this PR widens it to every enabled model on the path. ① normal path. Fix: make the sentence conditional on the resolved profile, or generalize it ("file-editing tools").

P3 — applyPatchProtocol: 'codex-v4a-freeform' on the DeepSeek adapter is a dead, misleading declaration (provider-registry.ts:945). resolveApplyPatchProfile only consults the field for the 'openai-structured' check; the freeform-vs-portable choice is driven by runtime.customTools, so DeepSeek always resolves portable-v4a despite the declaration. No runtime effect today (open-responses can't carry custom tools anyway — portable is correct), but the declaration invites a future reader to honor it and silently switch the wire format. Remove it, or make the freeform branch consult the field.

P3 — the Settings selector can't return a model to "automatic" (provider-capability-editor.tsx:100-112). The model is tri-state (list-default / explicit on / explicit off) but the UI is binary — once touched, the stored override can never go back to undefined, unlike the vision selector's auto/enabled/disabled at :83-98. A pinned true survives later removal from APPLY_PATCH_MODELS — intended for explicit choices, but the user had no way to say "default". Add an auto option writing undefined.

description: CODEX_PATCH_DESCRIPTION,
parameters:
profile.kind === 'codex-v4a-freeform' ? z.string() : z.object({ patch: z.string() }),
providerTool:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Code Mode + an enabled profile leaves the model with zero file-editing tools. Chain: this strips Write/Edit (:64) and marks the tool providerTool: {kind:'codex-apply-patch'} here (structured keeps openai-apply-patch). In Code Mode projectToolModePlan reduces the model-facing set to [exec] (ai-sdk-turn.ts:416-435), and nestableToolSnapshot excludes any tool with providerTool !== undefined (:456-462) — so apply_patch appears in neither the provider set nor exec's nested catalog. Repro: enable Code Mode in Settings, open a session on an openai-codex connection with a default-listed model — the model's only tool is exec with no editing capability, silently. Note the same hole exists on main for openai-structured, but this PR turns it on by default for the whole Codex OAuth fleet — the blast-radius expansion is this feature's. Fix belongs at the projection layer: either emit a nestable client-executed apply_patch (portable {patch: string} form, providerTool: undefined) when toolMode === 'code_mode' — the impl already accepts that shape (builtin-tools.ts:281-292) — or keep Write/Edit in the nested catalog when the profile's tool can't nest. ① normal user path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b5712c2. Code Mode now projects provider-native openai-apply-patch and codex-apply-patch tools into the portable client-executed { patch: string } shape before nestability filtering. This keeps the exclusive ApplyPatch editing surface while making apply_patch available through exec. I added end-to-end Code Mode coverage for both the OpenAI structured and Codex freeform paths.

@Astro-Han

Copy link
Copy Markdown
Contributor

A design thought prompted by the mid-session model-switch question — feel free to take or leave; none of this blocks the review above.

Consider making the patch-editing choice session-bound, stamped at creation, instead of per-model. toolMode already works exactly this way: chatDefaults.codeModeEnabled is a default that gets stamped onto the Session at creation (toolMode: 'code_mode' in the catalog coordinator) and never changes for the session's life. ApplyPatch is the same kind of thing — a tool-surface contract — so hanging it on the same axis would be more consistent.

What it buys:

  • The replay-normalization layer retires. normalizeApplyPatchReplayInput + applyPatchReplayFactText exist only because the profile can change mid-session across six direction pairs. If the session's editing mode is fixed at creation, the tool surface is constant and history is always self-consistent — no durable-fact downgrades, no profile-pair matrix.
  • The override matrix disappears. No more connection×model tri-state precedence to reason about; the model-capability list returns to being a purely internal authority consulted once at session creation to pick the default.
  • The Code Mode interaction becomes a creation-time check instead of a runtime collision (the P1 above): two session-level axes with a finite compatibility table, resolvable before the first turn.
  • The settings-UI question dissolves — there's no per-model knob to place.

The one thing it needs to answer — mid-session switch to a model that emits worse patches — is benign: patch capability is a quality gradient, not a hard failure, since the impl validates patches and returns tool errors on malformed input. The session keeps its promised surface; a weaker model just recovers more often. That seems strictly better than carrying mixed-history machinery to handle a case that's a soft degradation anyway.

If you keep the per-model design, two smaller suggestions: give the selector an auto tri-state (like the vision selector) so a choice can return to list-default, and move the control lower in the model settings — "which edit protocol this model speaks" is a second-order detail that currently sits at the core of the page.

Keep ApplyPatch available inside Code Mode through a portable nested projection, align the headless prompt with routed tools, remove the dead DeepSeek protocol declaration, and restore the model setting's automatic state.
@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

@Astro-Han Addressed the concrete review feedback in b5712c2:

  • P1: fixed the Code Mode projection and replied inline.
  • P2: generalized the headless-coding-v1 prompt so it refers to the available file-editing tool instead of naming Edit/Write; updated the frozen prompt contracts.
  • P3: removed the dead DeepSeek codex-v4a-freeform declaration. DeepSeek continues to use the portable function-call shape.
  • P3: restored the ApplyPatch setting to a true Automatic / Enabled / Disabled selector. Selecting Automatic clears the stored override; Storybook coverage exercises the round trip.

Verification: affected Core/Runtime/Runtime Host/Desktop typechecks passed; 132 focused runtime tests passed; the two headless prompt contract tests passed; Storybook production build and the ModelCapabilities / ModelParameterSave browser scenarios passed.

For the session-bound design suggestion: I am keeping the current per-model design in this PR for now while I think through model-switch and wire-protocol compatibility. I will treat that as a separate design decision rather than fold it into this review fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants