Repository navigation
[AIGTWY-4901] Only remove a Codex model ug wrote - #921
david-siqi-liu wants to merge 1 commit into
Conversation
b567051 to
b776f2f
Compare
b776f2f to
d79db10
Compare
d79db10 to
1d37724
Compare
1d37724 to
efc5f90
Compare
efc5f90 to
4ff9bec
Compare
4ff9bec to
6f3221d
Compare
725c829 to
dbbd42a
Compare
UG reviewReviewed Codex model ownership across configure, launch, and migration from managed-file snapshots. The migration can mistake a preserved admin model for a ug-owned pin. Blocker
Automated advisory review of |
dbbd42a to
c9eae01
Compare
c9eae01 to
7710c24
Compare
7710c24 to
d4a33a5
Compare
d4a33a5 to
e7733f2
Compare
|
Blocker (adopting a matching developer model on the first recorded write): fixed in e7733f2. The private-file write now always compares against the file as it was before the write, so a |
Codex's writer removed `model` and `model_reasoning_effort` from the private and OS-managed configs whenever ug had no managed default to pin, so a model the developer, an admin or another tool chose was deleted on every configure and launch. - Removals of `model` go through the per-file provenance record: ug restores the pre-ug value or removes it only when it wrote the current value and nobody changed it since. The record is updated only after a confirmed write. - `model_reasoning_effort` is never removed; ug never writes it. - `default_model` no longer clears preferences, so loading or saving state never rewrites the config. Launch still retires a model ug pinned. Co-authored-by: Isaac <no-reply@databricks.com>
e7733f2 to
1912551
Compare
|
Blocker (bootstrap adopting an admin model preserved in the last-applied snapshot): leaving as is. This only applies to the one-time upgrade bootstrap of a managed file that has no provenance record yet. Today's main removes the Codex |
…atabricks#1010) Standalone extract of the `default_model` part of databricks#921, so CUJ 6 (databricks#987) isn't blocked on the provenance stack. `codex.default_model()` called `clear_model_preferences` when there was no managed default in the state it was given. `ug configure` writes Codex's managed `model`, then `save_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 served `codex_extra` instead of the managed default `codex_primary`. Changes: - `src/ucode/agents/codex.py`: `default_model()` no longer calls `clear_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 cleared `model` and `model_reasoning_effort` now asserts it leaves the config byte-for-byte unchanged (the same flip databricks#921 makes). databricks#921 still carries the broader provenance-based fix. When it lands, it supersedes this line with no conflict in intent. Validation: `ruff check` and `ruff format --check` pass. Full `pytest`: 3201 passed; the only 2 failures are `test_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. Co-authored-by: Isaac <no-reply@databricks.com>
Codex's writer removed
modelandmodel_reasoning_effortfrom~/.codex/ucode.config.tomland/etc/codex/managed_config.tomlwhenever ug had no managed default to pin, on every configure, launch, and state load.Before / after
Before:
ugremoved the Codexmodel(andmodel_reasoning_effort) by name whenever it had no managed default, so a model the developer or an admin chose was deleted.After:
ugremoves or restoresmodelonly when the destination file's provenance record showsugwrote the current value. A developer-set model survives,model_reasoning_effortis never touched, and a modelugpinned for a prior workspace or managed default is still retired once the file has a record (or, for the managed file, a last-applied snapshot).modelis removed, or restored to its pre-ug value, only when the destination file's provenance record shows ug wrote the current value and nobody changed it since. A model ug pinned for a previous workspace or managed default is still retired, on configure and on launch.model_reasoning_effortis never removed; ug never writes it.default_model()only reads (that part already landed on its own in [AIGTWY-4881] Keep Codex's managed default model when ug saves state #1010). Launch still retires a model ug pinned, and forgets ownership of a model the developer has since changed.Ownership is recorded after a confirmed write only. A value either file already held is not adopted unless ug's record (or, for the managed file before the record existed, its last-applied snapshot) shows ug wrote it, a managed config that was already up to date adopts nothing new, and ug no longer backs up its own generated file as if it predated ug after a workspace switch.
For a managed config ug wrote before the record existed, ownership of
modelis bootstrapped from ug's last-applied snapshot, so an upgrading user's stale pin is still retired. A private config without a record keeps anymodelit has.Smart routing keeps its existing behavior.
This pull request and its description were written by Isaac.