Repository navigation
[AIGTWY-4881] Keep Codex's managed default model when ug saves state - #1010
Merged
Merged
Conversation
`codex.default_model()` called `clear_model_preferences`, so any state hydrate without the managed overlay (for example `save_state` right after `write_tool_config`) deleted the `model` ug had just written. Codex then started on whatever the gateway listed first instead of the admin's default. Looking up the default no longer rewrites config; launch still clears a stale pinned model. Co-authored-by: Isaac <no-reply@databricks.com>
david-siqi-liu
added a commit
that referenced
this pull request
Oct 6, 2026
david-siqi-liu
marked this pull request as ready for review
October 6, 2026 16:32
UG reviewReviewed the removal of configuration writes from Codex default-model lookup and the updated regression test, focusing on preserving managed model preferences during state saves. No actionable issues are evident in the supplied diff. No actionable findings. Automated advisory review of |
andy-xu-db
approved these changes
Oct 6, 2026
xsh310
pushed a commit
to xsh310/ucode
that referenced
this pull request
Oct 7, 2026
CUJ 6 (AIGTWY-4881, epic AIGTWY-4875) checks that a developer who receives successive admin configs ends up with exactly the latest one: no stale ug-managed values, and their own settings untouched. It runs live on main's dedicated-workspace CUJ framework (databricks#1000) with the TUI request recorder (databricks#994), against the CUJ 6 plan in the [UG Configure E2E doc](https://docs.google.com/document/d/1WKd1fdWD0Y4tAV1H9Si-SGx2UZFL7iZ2S3HtBjidmS0/edit). Workspace configs are read only, because parallel CI jobs share them. Each phase is the single published config of its own pre-provisioned workspace, and one shared home moves through them: - A: `dbc-8c38ce9b-634b`, CUJ 1's config plus the named MCP `ug_e2e.tools.fixture_reader` - B: `dbc-175bc4a3-a511`, Claude on the Bedrock MPS `ug_e2e.providers.bedrock`, Codex on UC discovery `ug_e2e.models`, Codex as the default agent - C: `dbc-998133ab-de86`, Codex only, on the static list `system.ai.gpt-5-6-luna` All three share one metastore, the `ug_e2e` fixtures, and one trace table. The test never writes workspace config; it checks each published config against the plan at the start and asserts it is unchanged at teardown. A to B to C also exercises ug's cross-workspace switch. Builds on databricks#1010 (merged), and on databricks#1004 (merged), whose `workspace_origin` keeps the TUI request recorder's `http://127.0.0.1:<port>` origin for the workspace API calls CUJ 6 routes through it. Changes: - `tests/e2e_cuj/test_cuj_repeated_config.py`: one class, phases as ordered class-scoped fixtures (`phase_a` -> `phase_b` -> `phase_c`) returning frozen evidence, 28 small tests on top. - Setup: seeds user-owned Claude and Codex prefs, a user stdio MCP, and a hand-written skill, with no ug state. - Readback: each phase workspace publishes exactly the plan's config (agents, model sources, defaults, headers, tracing, MCP selector). - Phase A: configures twice; identical settings and registrations, no duplicates; both agents complete a task with their phase headers and default models at gateway ingress (recorder) and in the native turn. - Phase B: Claude carries `databricks-model-provider-service` and no Phase A headers; Codex carries `databricks-model-service-parent-schema` and only the Phase B header; request and turn models match the sources. Bare `ug` in a real terminal launches Codex and completes a task on `ug_e2e.models.codex_primary`, and writes the Phase B header into `/etc/codex/managed_config.toml`. Static picker, family defaults, tracing keys, static Codex catalog, and the managed MCP are gone and stay gone after launches. Codex emits a span; Claude emits none. - Phase C: configures in a terminal (Phase B's terminal launch made ug own the machine-wide files) and clears Phase B's headers from `/etc`. Codex requests carry no Phase B headers, the picker is exactly Luna, Codex and bare `ug` both run Luna, `ug claude` is rejected with no inference request, and no Codex span is emitted while the Phase B span proves the trace query works. - Every phase: user prefs, MCP registrations, and skill files are unchanged, and each agent's registered user MCP still starts and lists its tool. - `tests/e2e_cuj/helpers/constants.py`: the CUJ 6 metastore fixtures. The Claude tracing keys come from `ucode.agents.claude.CLAUDE_OTEL_TRACE_ENV_KEYS`. - `tests/e2e_cuj/helpers/tui_request_recorder.py`: `retarget_session` is public so a terminal configure can be retargeted to the recorder. Known gaps (tracked in AIGTWY-4972): - Managed skills are not covered: UC skill bundles can't be uploaded on these workspaces' Default Storage catalog (Files API HTTP 501). Validation: ruff check and format pass; related unit tests pass. Live: `E2E CUJs` on this head passed 48 tests (CUJ 6's 28, plus CUJ 1, 2, 4, and 5) in 11m36s: https://github.com/databricks/unity-gateway/actions/runs/37567899055/job/112619786676 This pull request and its description were written by Isaac. Co-authored-by: Isaac <no-reply@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standalone extract of the
default_modelpart of #921, so CUJ 6 (#987) isn't blocked on the provenance stack.codex.default_model()calledclear_model_preferenceswhen there was no managed default in the state it was given.ug configurewrites Codex's managedmodel, thensave_state->hydrate_state(without the managed overlay) ->default_model_for_tool->codex.default_model()deleted it again. Codex then started on whatever model the gateway listed first instead of the admin's default. The live CUJ 6 run reproduced this: with a UC-location config, Codex servedcodex_extrainstead of the managed defaultcodex_primary.Changes:
src/ucode/agents/codex.py:default_model()no longer callsclear_model_preferences, so looking up the default never rewrites config. The launch path still calls it, so a stale model ug pinned is still retired at launch.tests/test_agent_codex.py: the test that asserted the lookup clearedmodelandmodel_reasoning_effortnow asserts it leaves the config byte-for-byte unchanged (the same flip [AIGTWY-4901] Only remove a Codex model ug wrote #921 makes).#921 still carries the broader provenance-based fix. When it lands, it supersedes this line with no conflict in intent.
Validation:
ruff checkandruff format --checkpass. Fullpytest: 3201 passed; the only 2 failures aretest_e2e_user_agent, which needs a live gateway and also fails locally on main. The updated test fails without the fix.This pull request and its description were written by Isaac.