Skip to content

Add managed-config serializer, validation, and per-agent model catalogs - #267

Open
tt-le wants to merge 1 commit into
mainfrom
tien/managed-setup-serializer
Open

Add managed-config serializer, validation, and per-agent model catalogs#267
tt-le wants to merge 1 commit into
mainfrom
tien/managed-setup-serializer

Conversation

@tt-le

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

Copy link
Copy Markdown
Collaborator

Changes

The admin-write foundation for the managed CodingAgentConfig, mirroring the developer-read side from #263. No CLI wiring and no network calls: pure functions plus tests. The interactive ucode setup wizard (#PR2) and ucode apply build on this.

managed_setup.py (new)

serialize_managed_config — ucode-native manifest → proto-JSON CodingAgentConfig, the exact inverse of normalize_managed_config. Builds each agent's AgentModelConfig oneof variant, which the server rejects when it doesn't match its agent: claude gets ClaudeDefaultModels family slots, codex gets no model list at all, and opencode/pi/gemini/copilot get a flat list. Output-only fields (workspace_id, timestamps, user ids) are never emitted.

validate_manifest — client-side pre-flight mirroring the handler's validateStoredConfig, so an admin sees problems before the round-trip rather than as an opaque INVALID_PARAMETER_VALUE. Also cross-checks configured models against the workspace's discovered inventory, skipped for agents routing through a Model Provider Service (those ids come from the provider's catalog).

— Per-agent model catalogs: claude → Claude only, gemini → Gemini only, codex → GPT + OSS, opencode/pi/copilot → everything discovered.

~/.ucode/managed-settings.json persistence (0600, workspace-scoped), distinct from managed-state.json: this is the authored manifest, that is the pulled one.

managed_config.py — makes the two enum maps public so the write side inverts them instead of restating them, so a new agent or MCP type is declared once and both directions pick it up. Mechanical rename otherwise.

Note on units

spending_percentage is a fraction in [0, 1], not a percent — the server validates that range. validate_manifest rejects e.g. 80 with a message naming the expected form, since the spec doc's prose says "80%".

Testing

uv run pytest — 1218 passed, 36 skipped on this commit alone. 77 new cases.

The load-bearing one is the round-trip property (serialize#263's normalize == identity), asserted over the full manifest, the minimal manifest, every known agent, and every MCP type. It pins read and write together so they cannot drift as the proto grows — which matters now that #265 consumes this same manifest shape.

This pull request and its description were written by Isaac.

Admin-write foundation for the managed CodingAgentConfig, mirroring the
developer-read side added in the parent commit. No CLI wiring and no network
calls: the interactive `ucode setup` wizard and `ucode apply` build on this in
follow-up changes.

`managed_setup.py`:
- `serialize_managed_config` — ucode-native manifest -> proto-JSON
  `CodingAgentConfig`, the exact inverse of `normalize_managed_config`. Builds
  each agent's `AgentModelConfig` oneof variant, which the server rejects when it
  doesn't match its agent: claude gets `ClaudeDefaultModels` family slots, codex
  gets no model list at all, and opencode/pi/gemini/copilot get a flat list.
  Output-only fields (workspace_id, timestamps, user ids) are never emitted.
- `validate_manifest` — client-side pre-flight mirroring the handler's
  `validateStoredConfig`, so an admin sees problems before the round-trip rather
  than as an opaque INVALID_PARAMETER_VALUE. Also cross-checks configured models
  against the workspace's discovered inventory, skipped for agents routing
  through a Model Provider Service (those ids come from the provider's catalog).
- Per-agent model catalogs: claude -> Claude only, gemini -> Gemini only,
  codex -> GPT + OSS, opencode/pi/copilot -> everything discovered.
- `~/.ucode/managed-settings.json` persistence (0600, workspace-scoped), distinct
  from `managed-state.json`: this is the authored manifest, that is the pulled one.

Note on units: `spending_percentage` is a fraction in [0, 1], not a percent — the
server validates that range. `validate_manifest` rejects e.g. 80 with a message
naming the expected form, since the spec doc's prose says "80%".

`managed_config.py`: makes the two enum maps public so the write side inverts them
instead of restating them — a new agent or MCP type is declared once and both
directions pick it up. Mechanical rename otherwise.

Tests: 77 cases. The load-bearing one is the round-trip property (serialize ->
normalize == identity), asserted over the full manifest, the minimal manifest,
every known agent, and every MCP type — it pins read and write together so they
cannot drift as the proto grows.

Co-authored-by: Isaac
@tt-le
tt-le force-pushed the tien/managed-setup-serializer branch from f4c989a to d820408 Compare August 5, 2026 17:50

@AarushiShah-db AarushiShah-db left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

few comments but stamping to unblock

from the provider's own catalog, not from UC model services, so the workspace inventory says
nothing about them.
"""
model_config = agent_config.get("model_config")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here can we also check against AGENT_MODEL_FAMILIES to make sure that the user isn't specifying a gpt model for claude code etc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll fold this one into the TUI flow pr

"pi": "pi",
"gemini": "gemini",
"copilot": "copilot",
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do we need this mapping if its anyways the same name for the key -> value? I think we have another list somewhere in the codebase with all the supported agents, can we use that? Just trying to reduce the number of places that we need to update everytime we want to support a new harness

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll remove, its pretty redundant

tt-le added a commit that referenced this pull request Aug 5, 2026
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
@tt-le
tt-le enabled auto-merge (squash) August 5, 2026 19:30
tt-le added a commit that referenced this pull request Aug 5, 2026
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
tt-le added a commit that referenced this pull request Aug 5, 2026
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
tt-le added a commit that referenced this pull request Aug 5, 2026
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
tt-le added a commit that referenced this pull request Aug 5, 2026
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
tt-le added a commit that referenced this pull request Aug 5, 2026
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
tt-le added a commit that referenced this pull request Aug 5, 2026
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
tt-le added a commit that referenced this pull request Aug 5, 2026
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
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