Skip to content

Commit d386d18

Browse files
committed
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
1 parent ca31bbb commit d386d18

1 file changed

Lines changed: 5 additions & 13 deletions

File tree

src/ucode/managed_setup.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,6 @@
4848
AGENT_TOOL_TO_ENUM: dict[str, str] = {tool: enum for enum, tool in AGENT_ENUM_TO_TOOL.items()}
4949
MCP_TAG_TO_TYPE_ENUM: dict[str, str] = {tag: enum for enum, tag in MCP_TYPE_ENUM_TO_TAG.items()}
5050

51-
# `AgentModelConfig` oneof variant key per agent. The server rejects a config whose variant doesn't
52-
# match its agent (`validateAgentModelConfig`), so this mapping is not cosmetic.
53-
_AGENT_MODEL_CONFIG_VARIANT: dict[str, str] = {
54-
"claude": "claude",
55-
"codex": "codex",
56-
"opencode": "opencode",
57-
"pi": "pi",
58-
"gemini": "gemini",
59-
"copilot": "copilot",
60-
}
61-
6251
# Agents whose model config carries a flat `models` list. Claude instead uses per-family slots
6352
# (`ClaudeDefaultModels`), and Codex has no model list at all — it selects exactly one model.
6453
_FLAT_MODEL_LIST_AGENTS = frozenset({"opencode", "pi", "gemini", "copilot"})
@@ -250,8 +239,11 @@ def _enabled_agent_payload(tool: str, agent_config: dict) -> dict:
250239
if isinstance(model_config, dict):
251240
body = _model_config_payload(tool, model_config)
252241
if body:
253-
variant = _AGENT_MODEL_CONFIG_VARIANT[tool]
254-
config["model_config"] = {variant: body}
242+
# The `AgentModelConfig` oneof field names are ucode's tool names verbatim (claude,
243+
# codex, opencode, pi, gemini, copilot), so the tool doubles as the variant key. The
244+
# server rejects a variant that doesn't match its agent (`validateAgentModelConfig`),
245+
# and the round-trip through `normalize_managed_config` pins that alignment in tests.
246+
config["model_config"] = {tool: body}
255247

256248
entry: dict = {"agent": AGENT_TOOL_TO_ENUM[tool]}
257249
if config:

0 commit comments

Comments
 (0)