Skip to content

Add ucode apply: publish the authored managed config - #271

Merged
AarushiShah-db merged 1 commit into
tien/managed-apply-clientsfrom
tien/managed-apply
Aug 7, 2026
Merged

Add ucode apply: publish the authored managed config#271
AarushiShah-db merged 1 commit into
tien/managed-apply-clientsfrom
tien/managed-apply

Conversation

@tt-le

@tt-le tt-le commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Changes

Stacked on #270 — review that first. This PR's diff includes the rest of the stack until those merge.

The publish step for the manifest ucode setup authors. Validates, shows what would change, confirms, then writes it to the workspace using the clients from #270. This completes the admin path: author (ucode setup) → review (ucode setup show) → publish (ucode apply).

ucode apply             # validate, preview, confirm, publish
ucode apply --dry-run   # validate and preview without writing
ucode apply --yes       # skip the confirmation (CI)

It updates in place rather than replacing

When the workspace already has a config, apply PATCHes it using its resource name, read back from the existing-config GET (normalize_managed_config preserves name).

Delete-then-create was the original plan, since v0's Create returns ALREADY_EXISTS. But that has a window where the workspace has no managed config — and if the create failed there, every developer would silently fall back to their own settings until someone re-ran the command. The server applies the update mask inside a single entity-store update, so a failed PATCH leaves the current config intact.

It is still a whole-manifest write: every path ucode owns is sent, so a field the admin dropped on a re-run is cleared rather than left behind.

It refuses to publish when it can't tell whether a config exists

A failed existence check is the one case where "just try the create" either duplicates or silently overwrites an admin's work. So an unreadable check is a hard error naming the reason, not a warning that proceeds.

Error mapping

_explain_publish_failure covers the failures an admin will actually hit:

Server error What ucode says
FEATURE_DISABLED Names codingAgentConfigCrudEnabled and who to ask — this is the likely first experience, since the CRUD flag is off by default
PERMISSION_DENIED Workspace admin is required to author the workspace's coding config
INVALID_PARAMETER_VALUE Passed through verbatim — the server names the offending field, which beats any paraphrase
ALREADY_EXISTS Shouldn't happen once we PATCH; suggests re-running

The admin gate and validation both run before anything is sent, so an invalid manifest or a non-admin costs no round trip.

Testing

uv run pytest — 1372 passed, 6 skipped. 19 new cases.

Mutation-verified three ways: always-create instead of PATCH, publishing despite an unreadable existence check, and publishing an invalid manifest each fail a specific test. Also covers that typer.Exit(0) isn't swallowed by the command's own RuntimeError handler — the same trap setup hit, where a successful run printed ERROR 0.

Verified against a staging workspace:

╭─────────────╮
│ ucode apply │
╰─────────────╯
✔ Databricks auth already available
✔ Admin permissions verified

Configuration summary
  Default agent: Claude Code
  Claude Code: system.ai.claude-sonnet-5 (per-user)
    opus: system.ai.claude-opus-5
    sonnet: system.ai.claude-sonnet-5
  Budget policy: my_policy_name
    at 50%: Claude Code / system.ai.claude-sonnet-5
    at 90%: Codex / system.ai.gpt-5-6-sol

! This will replace the config already published on <workspace>
  (currently: claude). Every developer picks the new one up on their
  next ucode run.
✔ Dry run: the config was validated but not published.

That exercised auth, the admin check, the summary, and the existing-config detection choosing the update path — without writing.

README

Documents the publish step and drops the "no partial update yet" caveat, which the PATCH path makes untrue.

Not included

A pre-publish validation probe (launching each agent to confirm its model actually answers) is deliberately out of scope — it needs its own retry UX and would roughly double this PR. Better as a follow-up or a --probe flag.

This pull request and its description were written by Isaac.

@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from d3ee7c2 to 13c66f0 Compare August 5, 2026 17:53
@tt-le
tt-le force-pushed the tien/managed-apply branch 2 times, most recently from c16711b to 8287258 Compare August 5, 2026 19:29
@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from b0ba733 to 7e2ec09 Compare August 5, 2026 19:58
@tt-le
tt-le force-pushed the tien/managed-apply branch from 8287258 to be99eed Compare August 5, 2026 19:58
@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from 7e2ec09 to 88f43af Compare August 5, 2026 20:49
@tt-le
tt-le force-pushed the tien/managed-apply branch from be99eed to d4fcec4 Compare August 5, 2026 20:49
@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from 88f43af to b8dff9e Compare August 5, 2026 21:03
@tt-le
tt-le force-pushed the tien/managed-apply branch from d4fcec4 to 1dd8c7e Compare August 5, 2026 21:03
@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from b8dff9e to 03a3f13 Compare August 5, 2026 21:12
@tt-le
tt-le force-pushed the tien/managed-apply branch from 1dd8c7e to 56a5e69 Compare August 5, 2026 21:12
@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from 03a3f13 to 32205ad Compare August 5, 2026 21:21
@tt-le
tt-le force-pushed the tien/managed-apply branch from 56a5e69 to dc06d66 Compare August 5, 2026 21:21
@tt-le
tt-le force-pushed the tien/managed-apply-clients branch from 32205ad to d386d18 Compare August 5, 2026 22:13
@tt-le
tt-le force-pushed the tien/managed-apply branch from dc06d66 to 0967828 Compare August 5, 2026 22:13
The publish step for the manifest `ucode setup` authors. Validates, shows what
would change, confirms, then writes it to the workspace via the clients added in
the parent commit.

Updates in place rather than replacing. When the workspace already has a config,
`apply` PATCHes it using its resource name (read back from the existing-config
GET, which `normalize_managed_config` preserves). Delete-then-create was the
original plan — v0's Create returns ALREADY_EXISTS — but it has a window where the
workspace has *no* managed config, and if the create failed there every developer
would silently fall back to their own settings until someone re-ran the command.
The server applies the update mask inside a single entity-store update, so a
failed PATCH leaves the current config intact. It is still a whole-manifest write:
every path ucode owns is sent, so a field the admin dropped on a re-run is cleared
rather than left behind.

Refuses to publish when it cannot tell whether a config already exists. A failed
existence check used to be the one case where "just try the create" would either
duplicate or silently overwrite an admin's work, so an unreadable check is a hard
error naming the reason rather than a warning.

`_explain_publish_failure` maps the failures an admin will actually hit.
FEATURE_DISABLED is the likely first experience — the CRUD flag is off by default —
so it names `codingAgentConfigCrudEnabled` instead of printing an HTTP 400.
INVALID_PARAMETER_VALUE is passed through verbatim: the server names the offending
field, which beats any paraphrase.

`--yes` skips the confirmation for CI; `--dry-run` validates and previews without
writing. The admin gate and validation both run before anything is sent, so an
invalid manifest or a non-admin costs no round trip.

README documents the publish step and drops the "no partial update yet" caveat,
which the PATCH path makes untrue.

Verified against eng-ml-inference.staging: `apply --dry-run` authenticated,
verified admin, rendered the summary, detected the existing config, and chose the
update path without writing.

Tests: 19 cases. Mutation-verified three ways — always-create instead of PATCH,
publishing despite an unreadable existence check, and publishing an invalid
manifest each fail a specific test. Also covers that `typer.Exit(0)` isn't caught
by the command's own RuntimeError handler, the same trap `setup` hit.

Co-authored-by: Isaac
@AarushiShah-db
AarushiShah-db merged commit f8fe6f1 into tien/managed-apply-clients Aug 7, 2026
2 checks passed
@AarushiShah-db
AarushiShah-db deleted the tien/managed-apply branch August 7, 2026 06:33
tt-le added a commit that referenced this pull request Aug 7, 2026
The publish step for the manifest `ucode setup` authors. Validates, shows what
would change, confirms, then writes it to the workspace via the clients added in
the parent commit.

Updates in place rather than replacing. When the workspace already has a config,
`apply` PATCHes it using its resource name (read back from the existing-config
GET, which `normalize_managed_config` preserves). Delete-then-create was the
original plan — v0's Create returns ALREADY_EXISTS — but it has a window where the
workspace has *no* managed config, and if the create failed there every developer
would silently fall back to their own settings until someone re-ran the command.
The server applies the update mask inside a single entity-store update, so a
failed PATCH leaves the current config intact. It is still a whole-manifest write:
every path ucode owns is sent, so a field the admin dropped on a re-run is cleared
rather than left behind.

Refuses to publish when it cannot tell whether a config already exists. A failed
existence check used to be the one case where "just try the create" would either
duplicate or silently overwrite an admin's work, so an unreadable check is a hard
error naming the reason rather than a warning.

`_explain_publish_failure` maps the failures an admin will actually hit.
FEATURE_DISABLED is the likely first experience — the CRUD flag is off by default —
so it names `codingAgentConfigCrudEnabled` instead of printing an HTTP 400.
INVALID_PARAMETER_VALUE is passed through verbatim: the server names the offending
field, which beats any paraphrase.

`--yes` skips the confirmation for CI; `--dry-run` validates and previews without
writing. The admin gate and validation both run before anything is sent, so an
invalid manifest or a non-admin costs no round trip.

README documents the publish step and drops the "no partial update yet" caveat,
which the PATCH path makes untrue.

Verified against eng-ml-inference.staging: `apply --dry-run` authenticated,
verified admin, rendered the summary, detected the existing config, and chose the
update path without writing.

Tests: 19 cases. Mutation-verified three ways — always-create instead of PATCH,
publishing despite an unreadable existence check, and publishing an invalid
manifest each fail a specific test. Also covers that `typer.Exit(0)` isn't caught
by the command's own RuntimeError handler, the same trap `setup` hit.

Co-authored-by: Isaac
tt-le added a commit that referenced this pull request Aug 7, 2026
* Add PATCH/DELETE transport and coding-agent-config CRUD clients

The write-side API plumbing for `ucode apply` (next change). No CLI wiring and no
interactive flow: transport helpers plus three clients, mirroring how the read
client landed.

`databricks.py` had `_http_get_json`/`_http_post_json` but no PATCH or DELETE.
Rather than a third and fourth near-copy of the same 40 lines of error handling,
the body-sending path is factored into `_http_send_json(method, ...)` and the
three verbs become thin wrappers. `_http_post_json`'s behavior is unchanged.

DELETE needed one real difference: its success response is
`google.protobuf.Empty`, which arrives as `{}` or an empty body depending on the
gateway. An empty body would otherwise be reported as "response was not valid
JSON", so `allow_empty_body` treats it as success. `delete_coding_agent_config`
returns only a reason — there is no payload worth handing back.

Clients for the three admin RPCs, all workspace-admin gated server-side:
- `create_coding_agent_config` — POST to the collection. v0 allows one config per
  workspace, so this returns ALREADY_EXISTS when one exists.
- `update_coding_agent_config` — PATCH the resource. Preferred over
  delete-then-create, which has a window where the workspace has *no* managed
  config: if the create failed, every developer would lose their config until
  someone re-ran the command. The server applies the mask inside a single
  entity-store update, so a failed write leaves the old config intact.
- `delete_coding_agent_config` — DELETE by resource name.

`MANAGED_CONFIG_UPDATE_MASK_PATHS` is every field ucode's manifest can set. The
server requires a non-empty mask and rejects paths outside its mutable set; this
is that set minus what ucode doesn't author — `budget_id` (deprecated for
`budget_policy.budget_id`, and rejected on write) and `default_options`/`tiers`
(the legacy model-only shape). Sending every path ucode owns, not just the
populated ones, is what lets a re-run *clear* a field the admin removed: the
server merges per path, so an omitted path leaves the old value in place.

`_coding_agent_config_url` joins on the API root rather than the collection URL,
since the resource name already carries the `coding-agent-configs/` segment and
would otherwise be duplicated.

Tests: 15 cases. The mask is checked against `serialize_managed_config`'s actual
output rather than a restated list, so adding a manifest field fails the test
instead of shipping a mask that cannot clear it. Mutation-verified three ways:
dropping `allow_empty_body`, dropping the `update_mask`, and dropping one mask
path each fail a specific test.

Co-authored-by: Isaac

* setup: require a UUID budget id, and index tiers the way the server does

Three gaps found by reading the server-side validation this manifest is written
for (universe #2365441), all reachable through `--from-file` even though the
wizard can't produce them.

`budget_policy.budget_id` must parse as a UUID. The handler requires it, and
until now ucode only checked non-empty — so a hand-written manifest carrying
`"budget_id": "eng-budget"` passed local validation and failed at the API with an
INVALID_PARAMETER_VALUE. Local pre-flight exists precisely to spend the round
trip on real problems. The message names `budget_configuration_id` so an admin
knows where to get a valid one.

Tier positions are now reported 0-based. The server indexes with `zipWithIndex`,
so ucode's `tiers[1]` and the API's `tiers[0]` described the same tier — an admin
reconciling the two messages would be looking at the wrong one.

Added a test that the deprecated top-level `CodingAgentConfig.budget_id` (field 3)
is never emitted, even when a hand-written manifest sets it. The serializer
already only writes `budget_policy.budget_id`; the handler rejects the top-level
field, so this pins behavior that is currently correct by construction rather
than by intent.

Test fixtures used short placeholders (`"b"`, `"budget-1"`) where a real
`budget_configuration_id` would be, so those are now UUIDs — 20 occurrences
across the two files. `list_workspace_budgets` only ever returns real ones, so
the fixtures were describing input the wizard can't produce.

Tests: +6. Mutation-verified: dropping the UUID check fails four cases, reverting
to 1-based indices fails `test_tier_positions_are_reported_zero_based`, and
emitting the top-level `budget_id` fails
`test_a_manifest_carrying_a_top_level_budget_id_still_omits_it`.

Co-authored-by: Isaac

* setup: drop the agent -> oneof-variant identity map

Review feedback on #267: `_AGENT_MODEL_CONFIG_VARIANT` mapped each agent to its
`AgentModelConfig` oneof key, but the proto's field names are ucode's tool names
verbatim — claude, codex, opencode, pi, gemini, copilot — so every entry mapped a
name to itself. One use site, so the tool now serves as the key directly.

The dict's only other effect was a KeyError on an unknown agent, which was already
unreachable: `serialize_managed_config` filters to `tool in AGENT_TOOL_TO_ENUM`
before calling this, and the `AGENT_TOOL_TO_ENUM[tool]` lookup two lines down would
raise first anyway.

No new test. The variant keys are already covered — hard-coding the wrong one fails
`test_codex_model_config_has_no_model_list` and
`test_flat_list_agents_use_repeated_models`, and the round-trip through
`normalize_managed_config` asserts the alignment for every agent.

The other half of that review comment — validating a model against its agent's
dialect, so a manifest can't pin a GPT id for Claude Code — is deliberately not
here. It turned out to need a decision rather than a patch: the agent -> families
mapping already exists twice (`agents._TOOL_DISCOVERY_SOURCES` and
`managed_setup._AGENT_MODEL_FAMILIES`) and the two disagree for codex, copilot,
opencode, and pi. Picking one requires checking each agent's own writer, and the
likely outcome is that this module's opencode entry is wrong — it lists `codex`
while `build_opencode_base_urls` serves no OpenAI route — which would be a picker
bug in the wizard, not a refactor. Landing that separately.

Co-authored-by: Isaac

* Add `ucode apply`: publish the authored managed config (#271)

The publish step for the manifest `ucode setup` authors. Validates, shows what
would change, confirms, then writes it to the workspace via the clients added in
the parent commit.

Updates in place rather than replacing. When the workspace already has a config,
`apply` PATCHes it using its resource name (read back from the existing-config
GET, which `normalize_managed_config` preserves). Delete-then-create was the
original plan — v0's Create returns ALREADY_EXISTS — but it has a window where the
workspace has *no* managed config, and if the create failed there every developer
would silently fall back to their own settings until someone re-ran the command.
The server applies the update mask inside a single entity-store update, so a
failed PATCH leaves the current config intact. It is still a whole-manifest write:
every path ucode owns is sent, so a field the admin dropped on a re-run is cleared
rather than left behind.

Refuses to publish when it cannot tell whether a config already exists. A failed
existence check used to be the one case where "just try the create" would either
duplicate or silently overwrite an admin's work, so an unreadable check is a hard
error naming the reason rather than a warning.

`_explain_publish_failure` maps the failures an admin will actually hit.
FEATURE_DISABLED is the likely first experience — the CRUD flag is off by default —
so it names `codingAgentConfigCrudEnabled` instead of printing an HTTP 400.
INVALID_PARAMETER_VALUE is passed through verbatim: the server names the offending
field, which beats any paraphrase.

`--yes` skips the confirmation for CI; `--dry-run` validates and previews without
writing. The admin gate and validation both run before anything is sent, so an
invalid manifest or a non-admin costs no round trip.

README documents the publish step and drops the "no partial update yet" caveat,
which the PATCH path makes untrue.

Verified against eng-ml-inference.staging: `apply --dry-run` authenticated,
verified admin, rendered the summary, detected the existing config, and chose the
update path without writing.

Tests: 19 cases. Mutation-verified three ways — always-create instead of PATCH,
publishing despite an unreadable existence check, and publishing an invalid
manifest each fail a specific test. Also covers that `typer.Exit(0)` isn't caught
by the command's own RuntimeError handler, the same trap `setup` hit.

Co-authored-by: Isaac

* apply: validate against the workspace's full model listing

`ucode apply` rejected a model `ucode setup` had just offered:

    claude: model 'system.ai.claude-opus-4-8' is not available on this workspace.

`state["claude_models"]` holds only the newest id per family, because the launch
path pins one model per family alias. The wizard deliberately offers the older
versions too — pinning `default_opus_model` to a known-good `claude-opus-4-8` is a
normal thing for an admin to want — and stashes the full listing on
`state["all_claude_models"]` so validation recognizes them.

That stash is never persisted. `setup` saves the manifest, not the state, so a
separate `apply` process starts from a fresh `load_state()` without it and
validates against the narrow per-family inventory. The failure lands at the very
end of the flow, naming a model the wizard itself had listed a moment earlier.

`apply` now re-fetches the listing instead of trusting what `setup` left behind,
which also covers a hand-edited or `--from-file` manifest authored on another
machine. Best-effort: a failed listing leaves validation on the narrower inventory
rather than blocking a publish on a transient API error. `ensure_databricks_auth`
moves above validation since the listing needs a token; nothing is written until
well after.

Aarushi predicted this failure, and its exact message, reviewing the invariant on
`_claude_candidates` in #268. The fix there documented and tested the invariant
within one process — both paths satisfied it — and so missed that `setup` and
`apply` are two processes.

Tests: +2, covering the older-version publish and that a failed fetch still
publishes. Mutation-verified: validating against bare `state` reproduces the
original error.

Co-authored-by: Isaac

* apply: send update_mask as a query param, not in the body

The PATCH was rejected by the server it was written for:

    HTTP 400 INVALID_PARAMETER_VALUE: Field 'update_mask' is required and must
    contain at least one subfield with a non-default value!

Two mistakes, both visible in the RPC's HTTP binding (universe
ai-gateway-api/api/proto/service.proto:319):

    patch: "/ai-gateway/v2/{coding_agent_config.name=coding-agent-configs/*}"
    body: "coding_agent_config"

`body: "coding_agent_config"` means the config *is* the entire request body, so
there is nowhere in it for a sibling `update_mask` — the one we nested was parsed
as an unknown config field, leaving the mask genuinely absent. It belongs in the
query string.

And `update_mask` is a `google.protobuf.FieldMask`, whose JSON and query form is a
single comma-separated string, not the `{"paths": [...]}` object we sent.

`name` stays in the body: the path template reads it from the config.

The mask contents were right, and the 15 tests covering them all passed — they
asserted on `payload["update_mask"]["paths"]`, which is exactly the shape the
server rejects. A unit test that mirrors the client's own assumption cannot catch
a wire-format error; this was only ever going to surface against a real workspace.

Tests: the update test now asserts the mask arrives in the query string, as one
comma-separated FieldMask, and is absent from the body. Mutation-verified:
restoring the nested-object form fails it.

Co-authored-by: Isaac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants